git: a7c9e79f70e8 - stable/12 - pgrp: Prevent use after free.

[Konstantin Belousov]

The branch stable/12 has been updated by kib:

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

commit a7c9e79f70e865909970aafa4a1ca9f5e50b168d
Author:     Konstantin Belousov <kib at FreeBSD.org>
AuthorDate: 2020-12-31 13:44:32 +0000
Commit:     Konstantin Belousov <kib at FreeBSD.org>
CommitDate: 2021-02-09 08:35:28 +0000

    pgrp: Prevent use after free.
    
    Tested by:      pho
    
    (cherry picked from commit ef739c7373d8b3833979ad471b31cb9e215411fd)
---
 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 04d82d30f8c2..4a202f0a7411 100644
--- a/sys/kern/kern_proc.c
+++ b/sys/kern/kern_proc.c
@@ -103,7 +103,6 @@ SDT_PROBE_DEFINE3(proc, , dtor, return, "struct proc *", "int", "void *");
 SDT_PROBE_DEFINE3(proc, , init, entry, "struct proc *", "int", "int");
 SDT_PROBE_DEFINE3(proc, , init, return, "struct proc *", "int", "int");
 
-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");
@@ -117,6 +116,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);
@@ -137,6 +137,7 @@ struct sx __exclusive_cache_line allproc_lock;
 struct sx __exclusive_cache_line proctree_lock;
 struct mtx __exclusive_cache_line ppeers_lock;
 uma_zone_t proc_zone;
+uma_zone_t pgrp_zone;
 
 /*
  * The offset of various fields in struct proc and struct thread.
@@ -194,6 +195,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();
 }
 
@@ -297,6 +300,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);
+}
+
 /*
  * Is p an inferior of the current process?
  */
@@ -476,8 +489,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
@@ -701,8 +712,7 @@ pgdelete(struct pgrp *pgrp)
 		tty_rel_pgrp(tp, pgrp);
 	}
 
-	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 a62b6f76da74..0f4814c59e78 100644
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -325,7 +325,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);
@@ -343,10 +343,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);
 }
@@ -385,7 +383,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) {
@@ -448,8 +446,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 a12646c951c5..9784d26a1215 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -841,7 +841,6 @@ struct proc {
 
 #ifdef MALLOC_DECLARE
 MALLOC_DECLARE(M_PARGS);
-MALLOC_DECLARE(M_PGRP);
 MALLOC_DECLARE(M_SESSION);
 MALLOC_DECLARE(M_SUBPROC);
 #endif
@@ -1011,6 +1010,7 @@ extern struct proclist zombproc;	/* List of zombie 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. */