Re: enforcing RLIMIT_NPROC in setuid() ?

In article <fmb03n$k2t$1ger.gmane.org>,
Christos Zoulas <christosastron.com> wrote:
>In article <20080112061745.GA13745netbsd.org>,
>David Holland  <dholland-securitynetbsd.org> wrote:
>>On Thu, Jan 10, 2008 at 04:23:47PM -0500, Christos
Zoulas wrote:
>> > The biggest problem I see with the change is
that
>> > a process that did not exceed the quota can be
penalized about it.
>> > Consider the case where a root daemon forks,
runs setuid and sleeps
>> > bringing the user above the NPROC resource
limit. Then if a different
>> > shell process tries to exec, it will fail.
>>
>>One could mostly work around this by only checking
at exec time in
>>processes that have been previously marked PK_SUGID
(that covers
>>processes that shift down from root, right?) or are
about to be.
>
>That is a great idea!
>
>christos

And here's a patch:

Index: kern_exec.c
============================================================
=======
RCS file: /cvsroot/src/sys/kern/kern_exec.c,v
retrieving revision 1.266
diff -u -u -r1.266 kern_exec.c
--- kern_exec.c	3 Jan 2008 14:36:58 -0000	1.266
+++ kern_exec.c	12 Jan 2008 18:34:16 -0000
 -431,6
+431,18 
 	p = l->l_proc;
 
 	/*
+	 * Check if we have exceeded our number of processes
limit.
+	 * This is so that we handle the case where a root daemon
+	 * forked, ran setuid and is trying to exec. We don't do
+	 * the reference counting check in setuid() like other
OS's
+	 * because that would require code modifications in the
programs
+	 * that call setuid().
+	 */
+	if ((p->p_flag & PK_SUGID) &&
chgproccnt(kauth_cred_getuid(p1->p_cred),
+	    0) > p->p_rlimit[RLIMIT_NPROC].rlim_cur)
+		return EAGAIN;
+
+	/*
 	 * Drain existing references and forbid new ones.  The
process
 	 * should be left alone until we're done here.  This is
necessary
 	 * to avoid race conditions - e.g. in ptrace() - that
might allow