Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-03 Thread Tom Horsley
On Wed, 3 Jan 2018 09:21:16 -0800
Kees Cook wrote:

> The more interesting thing here is that secureexec is set for a
> process that ISN'T actually setuid. (ptrace of a setuid process). I
> think tha'ts the real bug, but not something I'm going to be able to
> fix quickly. So, for now, I want to revert this, then try to fix the
> weird case, and see if that breaks anyone, then fix this back to
> secureexec.

Certainly a program file that has capabilities attached to it
via "setcap" is intended to be treated just like setuid if
the capabilities it has are a superset of the capabilities
of the debugger. (I don't know if that is a useful info in this
case, but I thought I'd mention it :-).


Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-03 Thread Tom Horsley
On Wed, 3 Jan 2018 09:21:16 -0800
Kees Cook wrote:

> The more interesting thing here is that secureexec is set for a
> process that ISN'T actually setuid. (ptrace of a setuid process). I
> think tha'ts the real bug, but not something I'm going to be able to
> fix quickly. So, for now, I want to revert this, then try to fix the
> weird case, and see if that breaks anyone, then fix this back to
> secureexec.

Certainly a program file that has capabilities attached to it
via "setcap" is intended to be treated just like setuid if
the capabilities it has are a superset of the capabilities
of the debugger. (I don't know if that is a useful info in this
case, but I thought I'd mention it :-).


Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-03 Thread Serge E. Hallyn
Quoting Kees Cook (keesc...@chromium.org):
> On Tue, Jan 2, 2018 at 11:06 PM, Serge E. Hallyn  wrote:
> > On Tue, Jan 02, 2018 at 03:21:33PM -0800, Kees Cook wrote:
> >> This is a logical revert of:
> >>
> >> commit e37fdb785a5f ("exec: Use secureexec for setting dumpability")
> >>
> >> This weakens dumpability back to checking only for uid/gid changes in
> >> current (which is useless), but userspace depends on dumpability not
> >> being tied to secureexec.
> >>
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1528633
> >>
> >> Reported-by: Tom Horsley 
> >> Fixes: e37fdb785a5f ("exec: Use secureexec for setting dumpability")
> >> Cc: sta...@vger.kernel.org
> >> Signed-off-by: Kees Cook 
> >> ---
> >>  fs/exec.c | 9 +++--
> >>  1 file changed, 7 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/exec.c b/fs/exec.c
> >> index 5688b5e1b937..7eb8d21bcab9 100644
> >> --- a/fs/exec.c
> >> +++ b/fs/exec.c
> >> @@ -1349,9 +1349,14 @@ void setup_new_exec(struct linux_binprm * bprm)
> >>
> >>   current->sas_ss_sp = current->sas_ss_size = 0;
> >>
> >> - /* Figure out dumpability. */
> >> + /*
> >> +  * Figure out dumpability. Note that this checking only of current
> >> +  * is wrong, but userspace depends on it. This should be testing
> >> +  * bprm->secureexec instead.
> >> +  */
> >>   if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
> >> - bprm->secureexec)
> >> + !(uid_eq(current_euid(), current_uid()) &&
> >> +   gid_eq(current_egid(), current_gid(
> >
> > So what about the pdeath_signal?  Is that going to be another subtle
> > time-bomb?
> 
> pdeath_signal used another wrong method to set itself, but it was
> better than dumpable. I'd rather we leave it as-is, since I'd like to
> have everything controlled by secureexec.

Yes, but if there is some weird userspace out there that depends on
pdeath_signal handling in the same corner case, then we'll break that
just like we did, except it'll be even harder to track down, because
debugging a wrong pdeath_signal will be even more subtle, and it'll
fail only when it's supposed to be exiting...

> The more interesting thing here is that secureexec is set for a
> process that ISN'T actually setuid. (ptrace of a setuid process). I

yeah i'd like to find some time to track that down too.

> think tha'ts the real bug, but not something I'm going to be able to
> fix quickly. So, for now, I want to revert this, then try to fix the
> weird case, and see if that breaks anyone, then fix this back to
> secureexec.

That sounds good, I'm only saying that the core bug is the wrong setting
of secureexec, and you've switched both setting of pdeath and
dumpability to using secureexec, so it stands to reason that setting of
pdeath is still wrong in these cases.

-serge


Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-03 Thread Serge E. Hallyn
Quoting Kees Cook (keesc...@chromium.org):
> On Tue, Jan 2, 2018 at 11:06 PM, Serge E. Hallyn  wrote:
> > On Tue, Jan 02, 2018 at 03:21:33PM -0800, Kees Cook wrote:
> >> This is a logical revert of:
> >>
> >> commit e37fdb785a5f ("exec: Use secureexec for setting dumpability")
> >>
> >> This weakens dumpability back to checking only for uid/gid changes in
> >> current (which is useless), but userspace depends on dumpability not
> >> being tied to secureexec.
> >>
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1528633
> >>
> >> Reported-by: Tom Horsley 
> >> Fixes: e37fdb785a5f ("exec: Use secureexec for setting dumpability")
> >> Cc: sta...@vger.kernel.org
> >> Signed-off-by: Kees Cook 
> >> ---
> >>  fs/exec.c | 9 +++--
> >>  1 file changed, 7 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/exec.c b/fs/exec.c
> >> index 5688b5e1b937..7eb8d21bcab9 100644
> >> --- a/fs/exec.c
> >> +++ b/fs/exec.c
> >> @@ -1349,9 +1349,14 @@ void setup_new_exec(struct linux_binprm * bprm)
> >>
> >>   current->sas_ss_sp = current->sas_ss_size = 0;
> >>
> >> - /* Figure out dumpability. */
> >> + /*
> >> +  * Figure out dumpability. Note that this checking only of current
> >> +  * is wrong, but userspace depends on it. This should be testing
> >> +  * bprm->secureexec instead.
> >> +  */
> >>   if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
> >> - bprm->secureexec)
> >> + !(uid_eq(current_euid(), current_uid()) &&
> >> +   gid_eq(current_egid(), current_gid(
> >
> > So what about the pdeath_signal?  Is that going to be another subtle
> > time-bomb?
> 
> pdeath_signal used another wrong method to set itself, but it was
> better than dumpable. I'd rather we leave it as-is, since I'd like to
> have everything controlled by secureexec.

Yes, but if there is some weird userspace out there that depends on
pdeath_signal handling in the same corner case, then we'll break that
just like we did, except it'll be even harder to track down, because
debugging a wrong pdeath_signal will be even more subtle, and it'll
fail only when it's supposed to be exiting...

> The more interesting thing here is that secureexec is set for a
> process that ISN'T actually setuid. (ptrace of a setuid process). I

yeah i'd like to find some time to track that down too.

> think tha'ts the real bug, but not something I'm going to be able to
> fix quickly. So, for now, I want to revert this, then try to fix the
> weird case, and see if that breaks anyone, then fix this back to
> secureexec.

That sounds good, I'm only saying that the core bug is the wrong setting
of secureexec, and you've switched both setting of pdeath and
dumpability to using secureexec, so it stands to reason that setting of
pdeath is still wrong in these cases.

-serge


Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-03 Thread Laura Abbott

On 01/03/2018 09:21 AM, Kees Cook wrote:

On Wed, Jan 3, 2018 at 4:11 AM, Tom Horsley  wrote:

On Wed, 3 Jan 2018 01:04:44 -0600
Serge E. Hallyn wrote:


This weakens dumpability back to checking only for uid/gid changes in
current (which is useless), but userspace depends on dumpability not
being tied to secureexec.

https://bugzilla.redhat.com/show_bug.cgi?id=1528633

Reported-by: Tom Horsley 


Seems right, any chance we could get a tested-by: Tom?  (Did we already
get that?)


I didn't test it myself, but all I'd do is run the test program
I've attached to the bugzilla above which is trivial compared
to be learning how to patch and build kernels. So it would be
much simpler for someone with the kernel already built to
extract the tarball and type make :-).


This is what I did to verify it. Thank you very much for the test case!

-Kees



I ran the test case again and can confirm that it works. I didn't
get a chance to try the other test case I reported (coredumping
systemd units) but I pointed the reporter to the patch.

Thanks,
Laura


Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-03 Thread Laura Abbott

On 01/03/2018 09:21 AM, Kees Cook wrote:

On Wed, Jan 3, 2018 at 4:11 AM, Tom Horsley  wrote:

On Wed, 3 Jan 2018 01:04:44 -0600
Serge E. Hallyn wrote:


This weakens dumpability back to checking only for uid/gid changes in
current (which is useless), but userspace depends on dumpability not
being tied to secureexec.

https://bugzilla.redhat.com/show_bug.cgi?id=1528633

Reported-by: Tom Horsley 


Seems right, any chance we could get a tested-by: Tom?  (Did we already
get that?)


I didn't test it myself, but all I'd do is run the test program
I've attached to the bugzilla above which is trivial compared
to be learning how to patch and build kernels. So it would be
much simpler for someone with the kernel already built to
extract the tarball and type make :-).


This is what I did to verify it. Thank you very much for the test case!

-Kees



I ran the test case again and can confirm that it works. I didn't
get a chance to try the other test case I reported (coredumping
systemd units) but I pointed the reporter to the patch.

Thanks,
Laura


Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-03 Thread Kees Cook
On Tue, Jan 2, 2018 at 11:06 PM, Serge E. Hallyn  wrote:
> On Tue, Jan 02, 2018 at 03:21:33PM -0800, Kees Cook wrote:
>> This is a logical revert of:
>>
>> commit e37fdb785a5f ("exec: Use secureexec for setting dumpability")
>>
>> This weakens dumpability back to checking only for uid/gid changes in
>> current (which is useless), but userspace depends on dumpability not
>> being tied to secureexec.
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1528633
>>
>> Reported-by: Tom Horsley 
>> Fixes: e37fdb785a5f ("exec: Use secureexec for setting dumpability")
>> Cc: sta...@vger.kernel.org
>> Signed-off-by: Kees Cook 
>> ---
>>  fs/exec.c | 9 +++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/exec.c b/fs/exec.c
>> index 5688b5e1b937..7eb8d21bcab9 100644
>> --- a/fs/exec.c
>> +++ b/fs/exec.c
>> @@ -1349,9 +1349,14 @@ void setup_new_exec(struct linux_binprm * bprm)
>>
>>   current->sas_ss_sp = current->sas_ss_size = 0;
>>
>> - /* Figure out dumpability. */
>> + /*
>> +  * Figure out dumpability. Note that this checking only of current
>> +  * is wrong, but userspace depends on it. This should be testing
>> +  * bprm->secureexec instead.
>> +  */
>>   if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
>> - bprm->secureexec)
>> + !(uid_eq(current_euid(), current_uid()) &&
>> +   gid_eq(current_egid(), current_gid(
>
> So what about the pdeath_signal?  Is that going to be another subtle
> time-bomb?

pdeath_signal used another wrong method to set itself, but it was
better than dumpable. I'd rather we leave it as-is, since I'd like to
have everything controlled by secureexec.

The more interesting thing here is that secureexec is set for a
process that ISN'T actually setuid. (ptrace of a setuid process). I
think tha'ts the real bug, but not something I'm going to be able to
fix quickly. So, for now, I want to revert this, then try to fix the
weird case, and see if that breaks anyone, then fix this back to
secureexec.

-Kees

-- 
Kees Cook
Pixel Security


Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-03 Thread Kees Cook
On Wed, Jan 3, 2018 at 4:11 AM, Tom Horsley  wrote:
> On Wed, 3 Jan 2018 01:04:44 -0600
> Serge E. Hallyn wrote:
>
>> > This weakens dumpability back to checking only for uid/gid changes in
>> > current (which is useless), but userspace depends on dumpability not
>> > being tied to secureexec.
>> >
>> > https://bugzilla.redhat.com/show_bug.cgi?id=1528633
>> >
>> > Reported-by: Tom Horsley 
>>
>> Seems right, any chance we could get a tested-by: Tom?  (Did we already
>> get that?)
>
> I didn't test it myself, but all I'd do is run the test program
> I've attached to the bugzilla above which is trivial compared
> to be learning how to patch and build kernels. So it would be
> much simpler for someone with the kernel already built to
> extract the tarball and type make :-).

This is what I did to verify it. Thank you very much for the test case!

-Kees

-- 
Kees Cook
Pixel Security


Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-03 Thread Kees Cook
On Tue, Jan 2, 2018 at 11:06 PM, Serge E. Hallyn  wrote:
> On Tue, Jan 02, 2018 at 03:21:33PM -0800, Kees Cook wrote:
>> This is a logical revert of:
>>
>> commit e37fdb785a5f ("exec: Use secureexec for setting dumpability")
>>
>> This weakens dumpability back to checking only for uid/gid changes in
>> current (which is useless), but userspace depends on dumpability not
>> being tied to secureexec.
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1528633
>>
>> Reported-by: Tom Horsley 
>> Fixes: e37fdb785a5f ("exec: Use secureexec for setting dumpability")
>> Cc: sta...@vger.kernel.org
>> Signed-off-by: Kees Cook 
>> ---
>>  fs/exec.c | 9 +++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/exec.c b/fs/exec.c
>> index 5688b5e1b937..7eb8d21bcab9 100644
>> --- a/fs/exec.c
>> +++ b/fs/exec.c
>> @@ -1349,9 +1349,14 @@ void setup_new_exec(struct linux_binprm * bprm)
>>
>>   current->sas_ss_sp = current->sas_ss_size = 0;
>>
>> - /* Figure out dumpability. */
>> + /*
>> +  * Figure out dumpability. Note that this checking only of current
>> +  * is wrong, but userspace depends on it. This should be testing
>> +  * bprm->secureexec instead.
>> +  */
>>   if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
>> - bprm->secureexec)
>> + !(uid_eq(current_euid(), current_uid()) &&
>> +   gid_eq(current_egid(), current_gid(
>
> So what about the pdeath_signal?  Is that going to be another subtle
> time-bomb?

pdeath_signal used another wrong method to set itself, but it was
better than dumpable. I'd rather we leave it as-is, since I'd like to
have everything controlled by secureexec.

The more interesting thing here is that secureexec is set for a
process that ISN'T actually setuid. (ptrace of a setuid process). I
think tha'ts the real bug, but not something I'm going to be able to
fix quickly. So, for now, I want to revert this, then try to fix the
weird case, and see if that breaks anyone, then fix this back to
secureexec.

-Kees

-- 
Kees Cook
Pixel Security


Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-03 Thread Kees Cook
On Wed, Jan 3, 2018 at 4:11 AM, Tom Horsley  wrote:
> On Wed, 3 Jan 2018 01:04:44 -0600
> Serge E. Hallyn wrote:
>
>> > This weakens dumpability back to checking only for uid/gid changes in
>> > current (which is useless), but userspace depends on dumpability not
>> > being tied to secureexec.
>> >
>> > https://bugzilla.redhat.com/show_bug.cgi?id=1528633
>> >
>> > Reported-by: Tom Horsley 
>>
>> Seems right, any chance we could get a tested-by: Tom?  (Did we already
>> get that?)
>
> I didn't test it myself, but all I'd do is run the test program
> I've attached to the bugzilla above which is trivial compared
> to be learning how to patch and build kernels. So it would be
> much simpler for someone with the kernel already built to
> extract the tarball and type make :-).

This is what I did to verify it. Thank you very much for the test case!

-Kees

-- 
Kees Cook
Pixel Security


Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-03 Thread Tom Horsley
On Wed, 3 Jan 2018 01:04:44 -0600
Serge E. Hallyn wrote:

> > This weakens dumpability back to checking only for uid/gid changes in
> > current (which is useless), but userspace depends on dumpability not
> > being tied to secureexec.
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1528633
> > 
> > Reported-by: Tom Horsley   
> 
> Seems right, any chance we could get a tested-by: Tom?  (Did we already
> get that?)

I didn't test it myself, but all I'd do is run the test program
I've attached to the bugzilla above which is trivial compared
to be learning how to patch and build kernels. So it would be
much simpler for someone with the kernel already built to
extract the tarball and type make :-).


Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-03 Thread Tom Horsley
On Wed, 3 Jan 2018 01:04:44 -0600
Serge E. Hallyn wrote:

> > This weakens dumpability back to checking only for uid/gid changes in
> > current (which is useless), but userspace depends on dumpability not
> > being tied to secureexec.
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1528633
> > 
> > Reported-by: Tom Horsley   
> 
> Seems right, any chance we could get a tested-by: Tom?  (Did we already
> get that?)

I didn't test it myself, but all I'd do is run the test program
I've attached to the bugzilla above which is trivial compared
to be learning how to patch and build kernels. So it would be
much simpler for someone with the kernel already built to
extract the tarball and type make :-).


Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-02 Thread Serge E. Hallyn
On Tue, Jan 02, 2018 at 03:21:33PM -0800, Kees Cook wrote:
> This is a logical revert of:
> 
> commit e37fdb785a5f ("exec: Use secureexec for setting dumpability")
> 
> This weakens dumpability back to checking only for uid/gid changes in
> current (which is useless), but userspace depends on dumpability not
> being tied to secureexec.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1528633
> 
> Reported-by: Tom Horsley 
> Fixes: e37fdb785a5f ("exec: Use secureexec for setting dumpability")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Kees Cook 
> ---
>  fs/exec.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index 5688b5e1b937..7eb8d21bcab9 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1349,9 +1349,14 @@ void setup_new_exec(struct linux_binprm * bprm)
>  
>   current->sas_ss_sp = current->sas_ss_size = 0;
>  
> - /* Figure out dumpability. */
> + /*
> +  * Figure out dumpability. Note that this checking only of current
> +  * is wrong, but userspace depends on it. This should be testing
> +  * bprm->secureexec instead.
> +  */
>   if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
> - bprm->secureexec)
> + !(uid_eq(current_euid(), current_uid()) &&
> +   gid_eq(current_egid(), current_gid(

So what about the pdeath_signal?  Is that going to be another subtle
time-bomb?

>   set_dumpable(current->mm, suid_dumpable);
>   else
>   set_dumpable(current->mm, SUID_DUMP_USER);
> -- 
> 2.7.4
> 
> 
> -- 
> Kees Cook
> Pixel Security


Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-02 Thread Serge E. Hallyn
On Tue, Jan 02, 2018 at 03:21:33PM -0800, Kees Cook wrote:
> This is a logical revert of:
> 
> commit e37fdb785a5f ("exec: Use secureexec for setting dumpability")
> 
> This weakens dumpability back to checking only for uid/gid changes in
> current (which is useless), but userspace depends on dumpability not
> being tied to secureexec.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1528633
> 
> Reported-by: Tom Horsley 
> Fixes: e37fdb785a5f ("exec: Use secureexec for setting dumpability")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Kees Cook 
> ---
>  fs/exec.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index 5688b5e1b937..7eb8d21bcab9 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1349,9 +1349,14 @@ void setup_new_exec(struct linux_binprm * bprm)
>  
>   current->sas_ss_sp = current->sas_ss_size = 0;
>  
> - /* Figure out dumpability. */
> + /*
> +  * Figure out dumpability. Note that this checking only of current
> +  * is wrong, but userspace depends on it. This should be testing
> +  * bprm->secureexec instead.
> +  */
>   if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
> - bprm->secureexec)
> + !(uid_eq(current_euid(), current_uid()) &&
> +   gid_eq(current_egid(), current_gid(

So what about the pdeath_signal?  Is that going to be another subtle
time-bomb?

>   set_dumpable(current->mm, suid_dumpable);
>   else
>   set_dumpable(current->mm, SUID_DUMP_USER);
> -- 
> 2.7.4
> 
> 
> -- 
> Kees Cook
> Pixel Security


Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-02 Thread Serge E. Hallyn
On Tue, Jan 02, 2018 at 03:21:33PM -0800, Kees Cook wrote:
> This is a logical revert of:
> 
> commit e37fdb785a5f ("exec: Use secureexec for setting dumpability")
> 
> This weakens dumpability back to checking only for uid/gid changes in
> current (which is useless), but userspace depends on dumpability not
> being tied to secureexec.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1528633
> 
> Reported-by: Tom Horsley 

Seems right, any chance we could get a tested-by: Tom?  (Did we already
get that?)

> Fixes: e37fdb785a5f ("exec: Use secureexec for setting dumpability")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Kees Cook 
> ---
>  fs/exec.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index 5688b5e1b937..7eb8d21bcab9 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1349,9 +1349,14 @@ void setup_new_exec(struct linux_binprm * bprm)
>  
>   current->sas_ss_sp = current->sas_ss_size = 0;
>  
> - /* Figure out dumpability. */
> + /*
> +  * Figure out dumpability. Note that this checking only of current
> +  * is wrong, but userspace depends on it. This should be testing
> +  * bprm->secureexec instead.
> +  */
>   if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
> - bprm->secureexec)
> + !(uid_eq(current_euid(), current_uid()) &&
> +   gid_eq(current_egid(), current_gid(
>   set_dumpable(current->mm, suid_dumpable);
>   else
>   set_dumpable(current->mm, SUID_DUMP_USER);
> -- 
> 2.7.4
> 
> 
> -- 
> Kees Cook
> Pixel Security


Re: [PATCH] exec: Weaken dumpability for secureexec

2018-01-02 Thread Serge E. Hallyn
On Tue, Jan 02, 2018 at 03:21:33PM -0800, Kees Cook wrote:
> This is a logical revert of:
> 
> commit e37fdb785a5f ("exec: Use secureexec for setting dumpability")
> 
> This weakens dumpability back to checking only for uid/gid changes in
> current (which is useless), but userspace depends on dumpability not
> being tied to secureexec.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1528633
> 
> Reported-by: Tom Horsley 

Seems right, any chance we could get a tested-by: Tom?  (Did we already
get that?)

> Fixes: e37fdb785a5f ("exec: Use secureexec for setting dumpability")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Kees Cook 
> ---
>  fs/exec.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index 5688b5e1b937..7eb8d21bcab9 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1349,9 +1349,14 @@ void setup_new_exec(struct linux_binprm * bprm)
>  
>   current->sas_ss_sp = current->sas_ss_size = 0;
>  
> - /* Figure out dumpability. */
> + /*
> +  * Figure out dumpability. Note that this checking only of current
> +  * is wrong, but userspace depends on it. This should be testing
> +  * bprm->secureexec instead.
> +  */
>   if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
> - bprm->secureexec)
> + !(uid_eq(current_euid(), current_uid()) &&
> +   gid_eq(current_egid(), current_gid(
>   set_dumpable(current->mm, suid_dumpable);
>   else
>   set_dumpable(current->mm, SUID_DUMP_USER);
> -- 
> 2.7.4
> 
> 
> -- 
> Kees Cook
> Pixel Security