linux-kernel - Re: [PATCH] signal: Avoid corrupting si_pid and si_uid in do_notify

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 21 Apr 2020 17:08:40 +0200
From:   Christian Brauner <christian.brauner@...ntu.com>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     Linux Containers <containers@...ts.linux-foundation.org>,
        Christof Meerwald <cmeerw@...erw.org>,
        linux-kernel@...r.kernel.org, Oleg Nesterov <oleg@...hat.com>
Subject: Re: [PATCH] signal: Avoid corrupting si_pid and si_uid in
 do_notify_parent

On Tue, Apr 21, 2020 at 09:57:09AM -0500, Eric W. Biederman wrote:
> Christian Brauner <christian.brauner@...ntu.com> writes:
> 
> > On Mon, Apr 20, 2020 at 12:05:38PM -0500, Eric W. Biederman wrote:
> >> diff --git a/kernel/signal.c b/kernel/signal.c
> >> index 9899c5f91ee1..a88a89422227 100644
> >> --- a/kernel/signal.c
> >> +++ b/kernel/signal.c
> >> @@ -1993,8 +1993,12 @@ bool do_notify_parent(struct task_struct *tsk, int sig)
> >>  		if (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN)
> >>  			sig = 0;
> >>  	}
> >> +	/*
> >> +	 * Bypass send_signal as the si_pid and si_uid values have
> >> +	 * been generated in the parent's namespaces.
> >> +	 */
> >
> > At first I misread that comment as saying that we're skipping sending a
> > signal not that it relates to a specific function (and I won't admit that
> > I wrote a whole long paragraph on why I'm confused we're skipping
> > sending signals on invalid si_pid and si_uid...).
> 
> I have updated the comment to read:
> +       /*
> +        * Send with __send_signal as si_pid and si_uid are in the
> +        * parent's namespaces.
> +        */
> 
> That should be enough of a hint for someone to read the code and figure
> out what is going on.

Perfect, thanks!
Acked-by: Christian Brauner <christian.brauner@...ntu.com>

Christian