d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

pgrp: Prevent use after free.

Browse Source 
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
This commit is contained in:
Konstantin Belousov 2020-12-31 15:44:32 +02:00
parent e0d83cd3e4
commit ef739c7373
3 changed files with 21 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
sys/kern/kern_proc.c
View File

@ -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);
}

13
sys/kern/kern_prot.c
View File

@ -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 @@ sys_setpgid(struct thread *td, struct setpgid_args *uap)
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);
}

2
sys/sys/proc.h
View File

@ -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. */