Date: Sun, 10 Jan 2021 02:41:53 GMT From: Konstantin Belousov <kib@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: ef739c7373d8 - main - pgrp: Prevent use after free. Message-ID: <202101100241.10A2frrf057586@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by kib: URL: https://cgit.FreeBSD.org/src/commit/?id=ef739c7373d8b3833979ad471b31cb9e215411fd commit ef739c7373d8b3833979ad471b31cb9e215411fd Author: Konstantin Belousov <kib@FreeBSD.org> AuthorDate: 2020-12-31 13:44:32 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-01-10 02:41:19 +0000 pgrp: Prevent use after free. Often, we have a process locked and need to get locked process group. In this case, because progress group lock is before process lock, unlocking process allows the group to be freed. See for instance tty_wait_background(). Make pgrp structures allocated from nofree zone, and ensure type stability of the pgrp mutex. Reviewed by: jilles Tested by: pho MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D27871 --- sys/kern/kern_proc.c | 20 +++++++++++++++----- sys/kern/kern_prot.c | 13 +++++-------- sys/sys/proc.h | 2 +- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c index ea34631995b1..fab1df795799 100644 --- a/sys/kern/kern_proc.c +++ b/sys/kern/kern_proc.c @@ -98,7 +98,6 @@ __FBSDID("$FreeBSD$"); SDT_PROVIDER_DEFINE(proc); -MALLOC_DEFINE(M_PGRP, "pgrp", "process group header"); MALLOC_DEFINE(M_SESSION, "session", "session header"); static MALLOC_DEFINE(M_PROC, "proc", "Proc structures"); MALLOC_DEFINE(M_SUBPROC, "subproc", "Proc sub-structures"); @@ -112,6 +111,7 @@ static void fill_kinfo_thread(struct thread *td, struct kinfo_proc *kp, int preferthread); static void pgadjustjobc(struct pgrp *pgrp, bool entering); static void pgdelete(struct pgrp *); +static int pgrp_init(void *mem, int size, int flags); static int proc_ctor(void *mem, int size, void *arg, int flags); static void proc_dtor(void *mem, int size, void *arg); static int proc_init(void *mem, int size, int flags); @@ -133,6 +133,7 @@ struct sx __exclusive_cache_line proctree_lock; struct mtx __exclusive_cache_line ppeers_lock; struct mtx __exclusive_cache_line procid_lock; uma_zone_t proc_zone; +uma_zone_t pgrp_zone; /* * The offset of various fields in struct proc and struct thread. @@ -196,6 +197,8 @@ procinit(void) proc_zone = uma_zcreate("PROC", sched_sizeof_proc(), proc_ctor, proc_dtor, proc_init, proc_fini, UMA_ALIGN_PTR, UMA_ZONE_NOFREE); + pgrp_zone = uma_zcreate("PGRP", sizeof(struct pgrp), NULL, NULL, + pgrp_init, NULL, UMA_ALIGN_PTR, UMA_ZONE_NOFREE); uihashinit(); } @@ -299,6 +302,16 @@ proc_fini(void *mem, int size) #endif } +static int +pgrp_init(void *mem, int size, int flags) +{ + struct pgrp *pg; + + pg = mem; + mtx_init(&pg->pg_mtx, "process group", NULL, MTX_DEF | MTX_DUPOK); + return (0); +} + /* * PID space management. * @@ -570,8 +583,6 @@ enterpgrp(struct proc *p, pid_t pgid, struct pgrp *pgrp, struct session *sess) KASSERT(!SESS_LEADER(p), ("enterpgrp: session leader attempted setpgrp")); - mtx_init(&pgrp->pg_mtx, "process group", NULL, MTX_DEF | MTX_DUPOK); - if (sess != NULL) { /* * new session @@ -798,8 +809,7 @@ pgdelete(struct pgrp *pgrp) } proc_id_clear(PROC_ID_GROUP, pgrp->pg_id); - mtx_destroy(&pgrp->pg_mtx); - free(pgrp, M_PGRP); + uma_zfree(pgrp_zone, pgrp); sess_release(savesess); } diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c index 529a6de4b2c8..170e9598835e 100644 --- a/sys/kern/kern_prot.c +++ b/sys/kern/kern_prot.c @@ -333,7 +333,7 @@ sys_setsid(struct thread *td, struct setsid_args *uap) error = 0; pgrp = NULL; - newpgrp = malloc(sizeof(struct pgrp), M_PGRP, M_WAITOK | M_ZERO); + newpgrp = uma_zalloc(pgrp_zone, M_WAITOK); newsess = malloc(sizeof(struct session), M_SESSION, M_WAITOK | M_ZERO); sx_xlock(&proctree_lock); @@ -351,10 +351,8 @@ sys_setsid(struct thread *td, struct setsid_args *uap) sx_xunlock(&proctree_lock); - if (newpgrp != NULL) - free(newpgrp, M_PGRP); - if (newsess != NULL) - free(newsess, M_SESSION); + uma_zfree(pgrp_zone, newpgrp); + free(newsess, M_SESSION); return (error); } @@ -393,7 +391,7 @@ sys_setpgid(struct thread *td, struct setpgid_args *uap) error = 0; - newpgrp = malloc(sizeof(struct pgrp), M_PGRP, M_WAITOK | M_ZERO); + newpgrp = uma_zalloc(pgrp_zone, M_WAITOK); sx_xlock(&proctree_lock); if (uap->pid != 0 && uap->pid != curp->p_pid) { @@ -456,8 +454,7 @@ done: sx_xunlock(&proctree_lock); KASSERT((error == 0) || (newpgrp != NULL), ("setpgid failed and newpgrp is NULL")); - if (newpgrp != NULL) - free(newpgrp, M_PGRP); + uma_zfree(pgrp_zone, newpgrp); return (error); } diff --git a/sys/sys/proc.h b/sys/sys/proc.h index 33da4969c6ae..2a7f0740a0c3 100644 --- a/sys/sys/proc.h +++ b/sys/sys/proc.h @@ -864,7 +864,6 @@ struct proc { #ifdef MALLOC_DECLARE MALLOC_DECLARE(M_PARGS); -MALLOC_DECLARE(M_PGRP); MALLOC_DECLARE(M_SESSION); MALLOC_DECLARE(M_SUBPROC); #endif @@ -1022,6 +1021,7 @@ extern struct proclist allproc; /* List of all processes. */ extern struct proc *initproc, *pageproc; /* Process slots for init, pager. */ extern struct uma_zone *proc_zone; +extern struct uma_zone *pgrp_zone; struct proc *pfind(pid_t); /* Find process by id. */ struct proc *pfind_any(pid_t); /* Find (zombie) process by id. */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202101100241.10A2frrf057586>