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 for Android: free password hash cracker in your pocket

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

Message-ID: <871rog6g56.fsf@x220.int.ebiederm.org>
Date:   Tue, 21 Apr 2020 09:57:09 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Christian Brauner <christian.brauner@...ntu.com>
Cc:     linux-kernel@...r.kernel.org,
        Linux Containers <containers@...ts.linux-foundation.org>,
        Christof Meerwald <cmeerw@...erw.org>,
        Oleg Nesterov <oleg@...hat.com>
Subject: Re: [PATCH] signal: Avoid corrupting si_pid and si_uid in do_notify_parent

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.

Eric