aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKonstantin Belousov <kib@FreeBSD.org>2020-12-31 13:44:32 +0000
committerKonstantin Belousov <kib@FreeBSD.org>2021-01-10 02:41:19 +0000
commitef739c7373d8b3833979ad471b31cb9e215411fd (patch)
treeb7b120a59b16b815a47b9eab42948090e285f49f
parente0d83cd3e49f0b9e16bc82ced7bd3b0ef9aa6a71 (diff)
downloadsrc-ef739c7373d8b3833979ad471b31cb9e215411fd.tar.gz
src-ef739c7373d8b3833979ad471b31cb9e215411fd.zip
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
-rw-r--r--sys/kern/kern_proc.c20
-rw-r--r--sys/kern/kern_prot.c13
-rw-r--r--sys/sys/proc.h2
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. */