git: 3138392a46a4 - main - itimer: Serialize access to the p_itimers array

[Mark Johnston]

The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=3138392a46a4a8ecfb8e36e9970e88bbae9caed3

commit 3138392a46a4a8ecfb8e36e9970e88bbae9caed3
Author:     Mark Johnston <markj at FreeBSD.org>
AuthorDate: 2021-08-31 20:38:05 +0000
Commit:     Mark Johnston <markj at FreeBSD.org>
CommitDate: 2021-08-31 20:38:05 +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 at syzkaller.appspotmail.com
    Reported by:    syzbot+929be2f32503bbc3844f at syzkaller.appspotmail.com
    Reviewed by:    kib
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D31759
---
 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);
 	}
 }