Re: enforcing RLIMIT_NPROC in setuid() ?

[christos%astron.com@localhost (Christos Zoulas)]

In article <fmb03n$k2t$1%ger.gmane.org@localhost>,
Christos Zoulas <christos%astron.com@localhost> wrote:
>In article <20080112061745.GA13745%netbsd.org@localhost>,
>David Holland  <dholland-security%netbsd.org@localhost> 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