linux-kernel - Re: [RFC][PATCH 1/5] exit: kill struct waitid

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:   Thu, 25 Jul 2019 00:01:23 +0200
From:   Christian Brauner <christian@...uner.io>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        Oleg Nesterov <oleg@...hat.com>, Arnd Bergmann <arnd@...db.de>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Kees Cook <keescook@...omium.org>,
        Joel Fernandes <joel@...lfernandes.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Tejun Heo <tj@...nel.org>, David Howells <dhowells@...hat.com>,
        Jann Horn <jannh@...gle.com>,
        Andrew Lutomirski <luto@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Aleksa Sarai <cyphar@...har.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Android Kernel Team <kernel-team@...roid.com>
Subject: Re: [RFC][PATCH 1/5] exit: kill struct waitid_info

On Wed, Jul 24, 2019 at 10:37:34AM -0700, Linus Torvalds wrote:
> On Wed, Jul 24, 2019 at 7:47 AM Christian Brauner <christian@...uner.io> wrote:
> >
> > The code here uses a struct waitid_info to catch basic information about
> > process exit including the pid, uid, status, and signal that caused the
> > process to exit. This information is then stuffed into a struct siginfo
> > for the waitid() syscall. That seems like an odd thing to do. We can
> > just pass down a siginfo_t struct directly which let's us cleanup and
> > simplify the whole code quite a bit.
> 
> Ack. Except I'd like the commit message to explain where this comes
> from instead of that "That seems like an odd thing to do".

I'll respin and will add an explanation based on the info you gave
below. Thanks for providing the background on that!

Christian

> 
> The _original_ reason for "struct waitid_info" was that "siginfo_t" is
> huge because of all the insane padding that various architectures do.
> 
> So it was introduced by commit 67d7ddded322 ("waitid(2): leave copyout
> of siginfo to syscall itself") very much to avoid stack usage issues.
> And I quote:
> 
>     collect the information needed for siginfo into
>     a small structure (waitid_info)
> 
> simply because "sigset_t" was big.
> 
> But that size came from the explicit "pad it out to 128 bytes for
> future expansion that will never happen", and the kernel using the
> same exact sigset_t that was exposed to user space.
> 
> Then in commit 4ce5f9c9e754 ("signal: Use a smaller struct siginfo in
> the kernel") we got rid of the insane padding for in-kernel use,
> exactly because it causes issues like this.
> 
> End result: that "struct waitid_info" no longer makes sense. It's not
> appreciably smaller than kernel_siginfo_t is today, but it made sense
> at the time.
> 
>                  Linus