Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 7 Sep 2021 13:36:27 GMT
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 996a40d6ad5e - stable/13 - itimer: Serialize access to the p_itimers array
Message-ID:  <202109071336.187DaRvG008117@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=996a40d6ad5e06b55aa52960ca6195c0b5fd94c9

commit 996a40d6ad5e06b55aa52960ca6195c0b5fd94c9
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2021-08-31 20:38:05 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2021-09-07 13:36:19 +0000

    itimer: Serialize access to the p_itimers array
    
    Fix the following race between itimer_proc_continue() and process exit.
    
    itimer_proc_continue() may be called via realitexpire(), the real
    interval timer.  Note that exit1() drains this timer _after_ draining
    and freeing itimers.  Moreover, itimers_exit() is called without the
    process lock held; it only acquires the proc lock when deleting
    individual itimers, so once they are drained we free p->p_itimers
    without any synchronization.  Thus, itimer_proc_continue() may load a
    non-NULL p->p_itimers array and iterate over it after it has been freed.
    
    Fix the problem by using the process lock when clearing p->p_itimers, to
    synchronize with itimer_proc_continue().  Formally, accesses to this
    field should be protected by the process lock anyway, and since the
    array is allocated lazily this will not incur any overhead in the common
    case.
    
    Reported by:    syzbot+c40aa8bf54fe333fc50b@syzkaller.appspotmail.com
    Reported by:    syzbot+929be2f32503bbc3844f@syzkaller.appspotmail.com
    Reviewed by:    kib
    Sponsored by:   The FreeBSD Foundation
    
    (cherry picked from commit 3138392a46a4a8ecfb8e36e9970e88bbae9caed3)
---
 sys/kern/kern_time.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sys/kern/kern_time.c b/sys/kern/kern_time.c
index 323ef9a1f7a0..a52dc83e9b4c 100644
--- a/sys/kern/kern_time.c
+++ b/sys/kern/kern_time.c
@@ -1822,8 +1822,11 @@ itimers_event_exit_exec(int start_idx, struct proc *p)
 	}
 	if (its->its_timers[0] == NULL && its->its_timers[1] == NULL &&
 	    its->its_timers[2] == NULL) {
-		free(its, M_SUBPROC);
+		/* Synchronize with itimer_proc_continue(). */
+		PROC_LOCK(p);
 		p->p_itimers = NULL;
+		PROC_UNLOCK(p);
+		free(its, M_SUBPROC);
 	}
 }
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202109071336.187DaRvG008117>