Re: [kernel-hardening] Re: [PATCH] move RLIMIT_NPROC check fromset_user() to do_execve_common()

[NeilBrown]

On Fri, 15 Jul 2011 10:31:13 +0400 Vasiliy Kulikov <segoon@xxxxxxxxxxxx>
wrote:

> Neil,
> 
> On Fri, Jul 15, 2011 at 13:30 +1000, NeilBrown wrote:
> > I'm still bothers that the proposed patch can cause exec to fail for an
> > separate 'innocent' process.
> > It also seems to 'hide' the problem - just shuffling code around.
> > The comment in do_execve_common helps.  A similar comment in set_user
> > wouldn't hurt.
> > 
> > But what do you think of this.  It sure that only the process which ignored
> > the return value from setuid is inconvenienced.
> 
> I don't like it.  You're mixing the main problem and an RLIMIT check
> enforcement.  The main goal is denying setuid() to fail unless there is not
> enough privileges, RLIMIT in execve() is just an *attempt* to still count
> NPROC in *some* widespread cases.  But you're trying to fix setuid()
> where RLIMIT accounting is simple :\
> 
> Your patch doesn't address the core issue in this situation:
> 
>     setuid(); /* it fails because of RLIMIT */
>     do_some_fs();
>     execve();
> 
> do_some_fs() should be called ONLY if root is dropped.  In your scheme
> the process may interact with FS as root while thinking it is nonroot,
> which almost always leads to privilege escalation.
> 
> Thanks,
> 

How about this then?