Date: Fri, 7 Oct 2011 06:46:46 +0000 (UTC) From: Edward Tomasz Napierala <trasz@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r226092 - in stable/9/sys: kern sys Message-ID: <201110070646.p976kkm4097316@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: trasz Date: Fri Oct 7 06:46:46 2011 New Revision: 226092 URL: http://svn.freebsd.org/changeset/base/226092 Log: MFC r225938: Fix bug introduced in r225641, which would cause panic if racct_proc_fork() returned error -- the racct_destroy_locked() would get called twice. MFC r225940: Fix another bug introduced in r225641, which caused rctl to access certain fields in 'struct proc' before they got initialized in do_fork(). MFC r225944: Move some code inside the racct_proc_fork(); it spares a few lock operations and it's more logical this way. MFC r225981: Actually enforce limit for inheritable resources on fork. Approved by: re (kib) Modified: stable/9/sys/kern/kern_fork.c stable/9/sys/kern/kern_racct.c stable/9/sys/kern/kern_rctl.c stable/9/sys/sys/racct.h Directory Properties: stable/9/sys/ (props changed) stable/9/sys/amd64/include/xen/ (props changed) stable/9/sys/boot/ (props changed) stable/9/sys/boot/i386/efi/ (props changed) stable/9/sys/boot/ia64/efi/ (props changed) stable/9/sys/boot/ia64/ski/ (props changed) stable/9/sys/boot/powerpc/boot1.chrp/ (props changed) stable/9/sys/boot/powerpc/ofw/ (props changed) stable/9/sys/cddl/contrib/opensolaris/ (props changed) stable/9/sys/conf/ (props changed) stable/9/sys/contrib/dev/acpica/ (props changed) stable/9/sys/contrib/octeon-sdk/ (props changed) stable/9/sys/contrib/pf/ (props changed) stable/9/sys/contrib/x86emu/ (props changed) Modified: stable/9/sys/kern/kern_fork.c ============================================================================== --- stable/9/sys/kern/kern_fork.c Fri Oct 7 06:13:38 2011 (r226091) +++ stable/9/sys/kern/kern_fork.c Fri Oct 7 06:46:46 2011 (r226092) @@ -879,17 +879,6 @@ fork1(struct thread *td, int flags, int goto fail1; } -#ifdef RACCT - PROC_LOCK(newproc); - error = racct_add(newproc, RACCT_NPROC, 1); - error += racct_add(newproc, RACCT_NTHR, 1); - PROC_UNLOCK(newproc); - if (error != 0) { - error = EAGAIN; - goto fail1; - } -#endif - #ifdef MAC mac_proc_init(newproc); #endif @@ -939,6 +928,7 @@ fork1(struct thread *td, int flags, int if (flags & RFPROCDESC) procdesc_finit(newproc->p_procdesc, fp_procdesc); #endif + racct_proc_fork_done(newproc); return (0); } Modified: stable/9/sys/kern/kern_racct.c ============================================================================== --- stable/9/sys/kern/kern_racct.c Fri Oct 7 06:13:38 2011 (r226091) +++ stable/9/sys/kern/kern_racct.c Fri Oct 7 06:46:46 2011 (r226092) @@ -261,12 +261,8 @@ racct_alloc_resource(struct racct *racct } } -/* - * Increase allocation of 'resource' by 'amount' for process 'p'. - * Return 0 if it's below limits, or errno, if it's not. - */ -int -racct_add(struct proc *p, int resource, uint64_t amount) +static int +racct_add_locked(struct proc *p, int resource, uint64_t amount) { #ifdef RCTL int error; @@ -282,23 +278,35 @@ racct_add(struct proc *p, int resource, */ PROC_LOCK_ASSERT(p, MA_OWNED); - mtx_lock(&racct_lock); #ifdef RCTL error = rctl_enforce(p, resource, amount); if (error && RACCT_IS_DENIABLE(resource)) { SDT_PROBE(racct, kernel, rusage, add_failure, p, resource, amount, 0, 0); - mtx_unlock(&racct_lock); return (error); } #endif racct_alloc_resource(p->p_racct, resource, amount); racct_add_cred_locked(p->p_ucred, resource, amount); - mtx_unlock(&racct_lock); return (0); } +/* + * Increase allocation of 'resource' by 'amount' for process 'p'. + * Return 0 if it's below limits, or errno, if it's not. + */ +int +racct_add(struct proc *p, int resource, uint64_t amount) +{ + int error; + + mtx_lock(&racct_lock); + error = racct_add_locked(p, resource, amount); + mtx_unlock(&racct_lock); + return (error); +} + static void racct_add_cred_locked(struct ucred *cred, int resource, uint64_t amount) { @@ -559,6 +567,12 @@ racct_proc_fork(struct proc *parent, str PROC_LOCK(child); mtx_lock(&racct_lock); +#ifdef RCTL + error = rctl_proc_fork(parent, child); + if (error != 0) + goto out; +#endif + /* * Inherit resource usage. */ @@ -569,32 +583,14 @@ racct_proc_fork(struct proc *parent, str error = racct_set_locked(child, i, parent->p_racct->r_resources[i]); - if (error != 0) { - /* - * XXX: The only purpose of these two lines is - * to prevent from tripping checks in racct_destroy(). - */ - for (i = 0; i <= RACCT_MAX; i++) - racct_set_locked(child, i, 0); + if (error != 0) goto out; - } } -#ifdef RCTL - error = rctl_proc_fork(parent, child); - if (error != 0) { - /* - * XXX: The only purpose of these two lines is to prevent from - * tripping checks in racct_destroy(). - */ - for (i = 0; i <= RACCT_MAX; i++) - racct_set_locked(child, i, 0); - } -#endif + error = racct_add_locked(child, RACCT_NPROC, 1); + error += racct_add_locked(child, RACCT_NTHR, 1); out: - if (error != 0) - racct_destroy_locked(&child->p_racct); mtx_unlock(&racct_lock); PROC_UNLOCK(child); PROC_UNLOCK(parent); @@ -602,6 +598,24 @@ out: return (error); } +/* + * Called at the end of fork1(), to handle rules that require the process + * to be fully initialized. + */ +void +racct_proc_fork_done(struct proc *child) +{ + +#ifdef RCTL + PROC_LOCK(child); + mtx_lock(&racct_lock); + rctl_enforce(child, RACCT_NPROC, 0); + rctl_enforce(child, RACCT_NTHR, 0); + mtx_unlock(&racct_lock); + PROC_UNLOCK(child); +#endif +} + void racct_proc_exit(struct proc *p) { @@ -827,6 +841,11 @@ racct_proc_fork(struct proc *parent, str } void +racct_proc_fork_done(struct proc *child) +{ +} + +void racct_proc_exit(struct proc *p) { } Modified: stable/9/sys/kern/kern_rctl.c ============================================================================== --- stable/9/sys/kern/kern_rctl.c Fri Oct 7 06:13:38 2011 (r226091) +++ stable/9/sys/kern/kern_rctl.c Fri Oct 7 06:46:46 2011 (r226092) @@ -312,6 +312,16 @@ rctl_enforce(struct proc *p, int resourc if (link->rrl_exceeded != 0) continue; + /* + * If the process state is not fully initialized yet, + * we can't access most of the required fields, e.g. + * p->p_comm. This happens when called from fork1(). + * Ignore this rule for now; it will be processed just + * after fork, when called from racct_proc_fork_done(). + */ + if (p->p_state != PRS_NORMAL) + continue; + if (!ppsratecheck(&lasttime, &curtime, 10)) continue; @@ -335,6 +345,9 @@ rctl_enforce(struct proc *p, int resourc if (link->rrl_exceeded != 0) continue; + if (p->p_state != PRS_NORMAL) + continue; + buf = malloc(RCTL_LOG_BUFSIZE, M_RCTL, M_NOWAIT); if (buf == NULL) { printf("rctl_enforce: out of memory\n"); @@ -357,23 +370,15 @@ rctl_enforce(struct proc *p, int resourc if (link->rrl_exceeded != 0) continue; + if (p->p_state != PRS_NORMAL) + continue; + KASSERT(rule->rr_action > 0 && rule->rr_action <= RCTL_ACTION_SIGNAL_MAX, ("rctl_enforce: unknown action %d", rule->rr_action)); /* - * We're supposed to send a signal, but the process - * is not fully initialized yet, probably because we - * got called from fork1(). For now just deny the - * allocation instead. - */ - if (p->p_state != PRS_NORMAL) { - should_deny = 1; - continue; - } - - /* * We're using the fact that RCTL_ACTION_SIG* values * are equal to their counterparts from sys/signal.h. */ Modified: stable/9/sys/sys/racct.h ============================================================================== --- stable/9/sys/sys/racct.h Fri Oct 7 06:13:38 2011 (r226091) +++ stable/9/sys/sys/racct.h Fri Oct 7 06:46:46 2011 (r226092) @@ -137,6 +137,7 @@ void racct_create(struct racct **racctp) void racct_destroy(struct racct **racctp); int racct_proc_fork(struct proc *parent, struct proc *child); +void racct_proc_fork_done(struct proc *child); void racct_proc_exit(struct proc *p); void racct_proc_ucred_changed(struct proc *p, struct ucred *oldcred,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201110070646.p976kkm4097316>