Re: [PATCH v3 00/15] exec: Use sane stack rlimit under secureexec

From: Serge E. Hallyn
Date: Tue Jul 18 2017 - 23:22:11 EST


On Tue, Jul 18, 2017 at 03:25:21PM -0700, Kees Cook wrote:
> This series has grown... :P
>
> As discussed with Linus and Andy, we need to reset the stack rlimit
> before we do memory layouts when execing a privilege-gaining (e.g.
> setuid) program. To do this, we need to know the results of the
> bprm_secureexec hook before memory layouts. As it turns out, this
> can be made _mostly_ trivial by collapsing bprm_secureexec into
> bprm_set_creds.
>
> The LSMs using bprm_secureexec nearly always save state between
> bprm_set_creds and bprm_secureexec. In the face of multiple calls to
> bprm_set_creds (via prepare_binprm() calls from binfmt_script, etc),
> all LSMs except commoncap only pay attention to the first call, so
> that aligns well with collapsing bprm_secureexec into bprm_set_creds.
> The commoncaps, though, needs to check the _last_ bprm_set_creds, so
> this series just swaps one bprm flag for another (cap_effective is no
> longer needed to save state between bprm_set_creds and bprm_secureexec,
> but we do need to keep a separate state, so we add the cap_elevated flag).
>
> Once secureexec is available to setup_new_exec() before the memory
> layout, we can add an rlimit sanity-check for setuid execs. (With no
> need to clean up since we're past the point of no return.)
>
> Along the way, this fixes comments, renames a variable, and consolidates
> dumpability and pdeath_signal clearing, which includes some commit log
> archeology to examine the subtle differences between what we had and
> what we need.
>
> I'd appreciate some extra eyes on this to make sure this isn't broken
> in some special way. Looking at the diffstat, even after all my long
> comments, this is a net reduction in lines. :)
>
> Given this crosses a bunch of areas, I think this is likely best to
> go via the -mm tree, which is where nearly all of my prior exec work
> has lived too.
>
> Thanks!
>
> -Kees
> ----------------------------------------------------------------
> Kees Cook (15):
> binfmt: Introduce secureexec flag
> exec: Rename bprm->cred_prepared to called_set_creds
> apparmor: Refactor to remove bprm_secureexec hook
> selinux: Refactor to remove bprm_secureexec hook
> smack: Refactor to remove bprm_secureexec hook
> commoncap: Refactor to remove bprm_secureexec hook
> commoncap: Move cap_elevated calculation into bprm_set_creds
> LSM: drop bprm_secureexec hook
> exec: Correct comments about "point of no return"
> exec: Use secureexec for setting dumpability
> exec: Use secureexec for clearing pdeath_signal
> smack: Remove redundant pdeath_signal clearing
> exec: Consolidate dumpability logic
> exec: Use sane stack rlimit under secureexec
> exec: Consolidate pdeath_signal clearing

Thanks, the set looks good to me,

Acked-by: Serge Hallyn <serge@xxxxxxxxxx>

Have you had a chance to run the ltp caps tests against this?

-serge