Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common()

[Solar Designer]

Neil, Vasiliy -

On Fri, Jul 15, 2011 at 11:38:23AM +0400, Vasiliy Kulikov wrote:
> On Fri, Jul 15, 2011 at 17:06 +1000, NeilBrown wrote:
> > How about this then?
> 
> AFAIU, with this patch:
> 
> 1) setuid() doesn't fail in NPROC exceed case.
> 2) NPROC is forced on execve() after setuid().
> 3) execve() fails only if NPROC was exceeded during setuid() execution.
> 4) Other processes of the same user doesn't suffer from "occasional"
> execve() failures.
> 
> If it is correct, then I like the patch :)

I'm not convinced this has any advantages over the patch Vasiliy had
posted (which simply moved the RLIMIT_NPROC check), but I don't mind,
with one important correction:

Although in the primary use cases we're considering, setuid() is very
soon followed by execve(), this doesn't always have to be the case.
There may be cases where the process would sleep between these two calls
(for a variety of reasons).  It would be surprising to see a process
fail on execve() because of RLIMIT_NPROC when that limit had been
reached, say, days ago and is no longer reached at the time of execve().

Thus, if we go with Neil's proposal (adding/checking an extra flag), we
should also re-check the process count against RLIMIT_NPROC on execve(),
and fail the exec only when both the flag is set and the current process
count is excessive (still or again).

Also, if we go with a patch like this, it brings up the question of
whether the flag should be preserved or reset on fork().  I think it
should be preserved.

> It does RLIMIT_NPROC
> enforcement without mixing other execve() calls like -ow patch did.

"Mixing other execve() calls" (4 on the list above) is not obviously
undesirable.  Thus, either Vasiliy's original patch or Neil's patch with
the addition mentioned above would be fine by me.

Thanks,

Alexander
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/