Date: Wed, 7 Jul 2010 21:48:20 GMT From: Edward Tomasz Napierala <trasz@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 180612 for review Message-ID: <201007072148.o67LmKj5062253@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@180612?ac=10 Change 180612 by trasz@trasz_victim on 2010/07/07 21:47:39 container_join() needs to be able to return error, should the resources used by the joining container push resource usage in parent container above is limits. Make it so. Affected files ... .. //depot/projects/soc2009/trasz_limits/sys/kern/kern_container.c#7 edit .. //depot/projects/soc2009/trasz_limits/sys/kern/kern_fork.c#17 edit .. //depot/projects/soc2009/trasz_limits/sys/kern/kern_hrl.c#79 edit .. //depot/projects/soc2009/trasz_limits/sys/sys/container.h#5 edit .. //depot/projects/soc2009/trasz_limits/sys/sys/proc.h#23 edit Differences ... ==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_container.c#7 (text+ko) ==== @@ -57,7 +57,7 @@ struct mtx container_lock; MTX_SYSINIT(container_lock, &container_lock, "container lock", MTX_RECURSE); /* XXX: Make it non-recurseable later. */ -static void +static int container_add(struct container *dest, const struct container *src) { int i; @@ -69,10 +69,19 @@ ("resource usage propagation meltdown: dest < 0")); KASSERT(src->c_resources[i] >= 0, ("resource usage propagation meltdown: src < 0")); + /* + * XXX: Container hierarchy! + */ + /* + * XXX: Enforce limit here; if exceeded, undo everything + * and return error. + */ dest->c_resources[i] += src->c_resources[i]; KASSERT(dest->c_resources[i] >= 0, ("resource usage propagation meltdown: dest < 0 after addition")); } + + return (0); } static void @@ -90,15 +99,18 @@ KASSERT(src->c_resources[i] <= dest->c_resources[i], ("resource usage propagation meltdown: src > dest")); dest->c_resources[i] -= src->c_resources[i]; + /* + * XXX: Container hierarchy! + */ KASSERT(dest->c_resources[i] >= 0, ("resource usage propagation meltdown: dest < 0 after subtraction")); } } -void +int container_join(struct container *child, struct container *parent) { - int i; + int i, error; mtx_assert(&container_lock, MA_OWNED); KASSERT(child != NULL, ("child != NULL")); @@ -108,9 +120,12 @@ KASSERT(child->c_parents[i] != parent, ("container already joined")); if (child->c_parents[i] == NULL) { + error = container_add(parent, child); + if (error) + return (error); + child->c_parents[i] = parent; - container_add(parent, child); - return; + return (0); } } panic("container has too many parents"); @@ -362,13 +377,13 @@ } /* - * Inherit resource usage information and copy limits from the parent - * process to the child. + * Inherit resource usage information and containing containers + * from the parent process. */ -void +int container_proc_fork(struct proc *parent, struct proc *child) { - int i; + int i, error = 0; struct container *container; PROC_LOCK(parent); @@ -379,22 +394,44 @@ * Create container for the child process and inherit containing * containers from the parent. */ + bzero(&child->p_container, sizeof(child->p_container)); container_create(&child->p_container); for (i = 0; i <= CONTAINER_PARENTS_MAX; i++) { container = parent->p_container.c_parents[i]; if (container == NULL) continue; - container_join(&child->p_container, container); + error = container_join(&child->p_container, container); + if (error) { + container_destroy(&child->p_container); + goto out; + } } + /* + * Inherit resource usage. + */ for (i = 0; i <= RUSAGE_MAX; i++) { - if (parent->p_container.c_resources[i] != 0 && - container_resource_inheritable(i)) - rusage_set(child, i, - parent->p_container.c_resources[i]); + if (parent->p_container.c_resources[i] == 0 || + !container_resource_inheritable(i)) + continue; + + error = rusage_set(child, i, parent->p_container.c_resources[i]); + if (error) { + /* + * XXX: The only purpose of these two lines is to prevent from + * tripping checks in container_leave_parents(). + */ + for (i = 0; i <= RUSAGE_MAX; i++) + rusage_set(child, i, 0); + container_destroy(&child->p_container); + goto out; + } } +out: mtx_unlock(&container_lock); PROC_UNLOCK(child); PROC_UNLOCK(parent); + + return (error); } ==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_fork.c#17 (text+ko) ==== @@ -351,6 +351,15 @@ } /* + * Initialize resource container for the child process. + */ + error = container_proc_fork(p1, newproc); + if (error) { + error = EAGAIN; + goto fail; + } + + /* * Increment the count of procs running with this uid. Don't allow * a nonprivileged user to exceed their current limit. * @@ -739,11 +748,6 @@ } /* - * Initialize resource container for the child process. - */ - container_proc_fork(p1, p2); - - /* * Both processes are set up, now check if any loadable modules want * to adjust anything. * What if they have an error? XXX @@ -798,6 +802,7 @@ *procp = p2; return (0); fail: + container_proc_exit(newproc); sx_sunlock(&proctree_lock); if (ppsratecheck(&lastfail, &curfail, 1)) printf("maxproc limit exceeded by uid %i, please see tuning(7) and login.conf(5).\n", ==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_hrl.c#79 (text+ko) ==== @@ -1445,14 +1445,18 @@ void hrl_proc_init(struct proc *p) { + int error; struct ucred *cred = p->p_ucred; mtx_lock(&container_lock); container_create(&p->p_container); - container_join(&p->p_container, &cred->cr_ruidinfo->ui_container); - container_join(&p->p_container, &cred->cr_loginclass->lc_container); - container_join(&p->p_container, &cred->cr_prison->pr_container); + error = container_join(&p->p_container, &cred->cr_ruidinfo->ui_container); + KASSERT(error == 0, ("hrl_proc_init: better error handling needed")); + error = container_join(&p->p_container, &cred->cr_loginclass->lc_container); + KASSERT(error == 0, ("hrl_proc_init: better error handling needed")); + error = container_join(&p->p_container, &cred->cr_prison->pr_container); + KASSERT(error == 0, ("hrl_proc_init: better error handling needed")); mtx_unlock(&container_lock); } @@ -1520,7 +1524,8 @@ } container_leave(&p->p_container, &olduip->ui_container); - container_join(&p->p_container, &newuip->ui_container); + error = container_join(&p->p_container, &newuip->ui_container); + KASSERT(error == 0, ("hrl_proc_init: better error handling needed")); } if (newlc != oldlc) { LIST_FOREACH(link, &newlc->lc_container.c_rule_links, hrl_next) { @@ -1529,7 +1534,8 @@ } container_leave(&p->p_container, &oldlc->lc_container); - container_join(&p->p_container, &newlc->lc_container); + error = container_join(&p->p_container, &newlc->lc_container); + KASSERT(error == 0, ("hrl_proc_init: better error handling needed")); } if (newpr != oldpr) { LIST_FOREACH(link, &newpr->pr_container.c_rule_links, hrl_next) { @@ -1538,7 +1544,8 @@ } container_leave(&p->p_container, &oldpr->pr_container); - container_join(&p->p_container, &newpr->pr_container); + error = container_join(&p->p_container, &newpr->pr_container); + KASSERT(error == 0, ("hrl_proc_init: better error handling needed")); } mtx_unlock(&container_lock); ==== //depot/projects/soc2009/trasz_limits/sys/sys/container.h#5 (text+ko) ==== @@ -96,10 +96,10 @@ void container_create(struct container *container); void container_destroy(struct container *container); -void container_join(struct container *child, struct container *parent); +int container_join(struct container *child, struct container *parent); void container_leave(struct container *child, struct container *parent); +int container_proc_fork(struct proc *parent, struct proc *child); void container_proc_exit(struct proc *p); -void container_proc_fork(struct proc *parent, struct proc *child); #endif /* !_CONTAINER_H_ */ ==== //depot/projects/soc2009/trasz_limits/sys/sys/proc.h#23 (text+ko) ==== @@ -524,7 +524,6 @@ int p_boundary_count;/* (c) Num threads at user boundary */ int p_pendingcnt; /* how many signals are pending */ struct itimers *p_itimers; /* (c) POSIX interval timers. */ - struct container p_container; /* (*) resource usage accounting */ /* End area that is zeroed on creation. */ #define p_endzero p_magic @@ -558,6 +557,7 @@ LIST_HEAD(, mqueue_notifier) p_mqnotifier; /* (c) mqueue notifiers.*/ struct kdtrace_proc *p_dtrace; /* (*) DTrace-specific data. */ struct cv p_pwait; /* (*) wait cv for exit/exec */ + struct container p_container; /* (*) resource usage accounting */ }; #define p_session p_pgrp->pg_session
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201007072148.o67LmKj5062253>