Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Andy Lutomirski]

On Thu, Jul 6, 2017 at 5:38 AM, Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote:
> Kees Cook <keescook@xxxxxxxxxxxx> writes:
>
>> In an attempt to provide sensible rlimit defaults for setuid execs, this
>> inherits the namespace's init rlimits:
>>
>> $ ulimit -s
>> 8192
>> $ ulimit -s unlimited
>> $ /bin/sh -c 'ulimit -s'
>> unlimited
>> $ sudo /bin/sh -c 'ulimit -s'
>> 8192
>>
>> This is modified from Brad Spengler/PaX Team's hard-coded setuid exec
>> stack rlimit (8MB) in the last public patch of grsecurity/PaX based on
>> my understanding of the code. Changes or omissions from the original
>> code are mine and don't reflect the original grsecurity/PaX code.
>>
>> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
>> ---
>> Instead of copying all rlimits, we could also pick specific ones to copy
>> (e.g. RLIMIT_STACK, or ones from Andy's list) or exclude from copying
>> (probably better to blacklist than whitelist).
>>
>> I think this is the right way to find the ns init task, but maybe it
>> needs locking?
>> ---
>>  fs/exec.c | 34 ++++++++++++++++++++++++++++++----
>>  1 file changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/exec.c b/fs/exec.c
>> index 904199086490..80e8b2bd4284 100644
>> --- a/fs/exec.c
>> +++ b/fs/exec.c
>> @@ -1675,6 +1675,12 @@ static int exec_binprm(struct linux_binprm *bprm)
>>       return ret;
>>  }
>>
>> +static inline bool is_setuid_exec(struct linux_binprm *bprm)
>> +{
>> +     return (!uid_eq(bprm->cred->euid, current_euid()) ||
>> +             !gid_eq(bprm->cred->egid, current_egid()));
>> +}
>
> Awesome I can make an executable setuid to myself and get all of roots
> rlimits!
>
> Scratch inheritable rlimits as useful for any kind of policy decision.

Most of them are already fairly useless.  But it's a fair point.  We
don't want Jane to run a setuid-John program and thereby escape
whatever limits are imposed.  We don't really have a concept of
bona-fide-trustworthy-setuid-root, though.