Re: [4.11 regression] su / sudo doesn't work when enlightenment is running as the window manager

[Takashi Iwai]

On Mon, 22 May 2017 22:24:17 +0200,
Eric W. Biederman wrote:
> 
> Takashi Iwai <tiwai@xxxxxxx> writes:
> 
> > Hi Eric,
> >
> > we've got a bug report showing the weird behavior supposedly triggered
> > by your commit 20523132ec5d ("exec: Test the ptracer's saved cred to
> > see if the tracee can gain caps").  In short, user can't run su or
> > sudo any longer when a terminal is started from Enlighentment window
> > manager.
> >
> > Some details are found in openSUSE Bugzilla:
> >   https://bugzilla.suse.com/show_bug.cgi?id=1040041
> >
> > and more in gentoo forum and Enlightenment bug tracker:
> >   https://forums.gentoo.org/viewtopic-t-1063022-postdays-0-postorder-asc-start-0.html
> >   https://phab.enlightenment.org/T5470
> >
> > As the bug seems solely in Enlightenment, it might that be some its
> > specific patch is broken.  But a regression is a regression, after
> > all...
> >
> > Could you take a look at the issue?
> 
> I will.
> 
> I am wondering if the bisect did not quite go back far enough as that
> change really should have been had no effect and it was the introduction
> of the ptracer_capable test that is causing problems.
> 
> To be able to think this through clearly I really need to understand
> what enlightenment-start is doing.
> 
> At first glance the reported behavior does seem corect.  If you are not
> root and you are ptracing a setuid-root application it should not have
> be run setuid.
> 
> So either I made a stilly mistake somewhere or enlightenment is doing
> something problematic.  The ugly possibility is that I might have closed
> a security hole they were depending upon by accident.
> 
> I took a quick look at what I think is the code to enlightment_start.
> AKA src/bin/e_start_main.c
> 
> And all it does is:
> 	pid = fork();
>         if (pid == 0) {
>         	/* child */
>                 ptrace(PT_TRACE_ME, 0, NULL);
>         } else {
>         	ptrace(PT_ATTACH, pid, NULL, NULL);
>                 ...;
>         }
> 
> So the tracer_cred should be the same on either path.
> 
> So if someone who understands what enlightenment is doing could boil
> this down or otherwise help me understand exactly what kind of ptrace
> interaction is causing this problem that would really help.
> 
> Hmm. You know I think I know what is going on and it is a bit
> embarrasing:
> 
> Can someone please test this change?  I am guessing I just forgot to zero
> ptracer_cred on fork...  Why ptrace_init_task lives in a header
> instead of in kernel/fork.c I do not know.

Thanks for a prompt reaction!

I'll prepare a test kernel and ask reporters giving it a try.


Takashi

> 
> diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
> index 422bc2e4cb6a..a31442c4b272 100644
> --- a/include/linux/ptrace.h
> +++ b/include/linux/ptrace.h
> @@ -202,6 +202,7 @@ static inline void ptrace_init_task(struct task_struct *child, bool ptrace)
>         INIT_LIST_HEAD(&child->ptraced);
>         child->jobctl = 0;
>         child->ptrace = 0;
> +       child->ptracer_cred = NULL;
>         child->parent = child->real_parent;
>  
>         if (unlikely(ptrace) && current->ptrace) {
> 
> Eric
>