linux-kernel - Re: [PATCH 7/7] signal: Remove kernel interal si

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, 18 Jul 2017 09:57:09 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        Oleg Nesterov <oleg@...hat.com>,
        Andrei Vagin <avagin@...tuozzo.com>,
        Thomas Gleixner <tglx@...utronix.de>, Greg KH <greg@...ah.com>,
        Andrey Vagin <avagin@...nvz.org>,
        Serge Hallyn <serge@...lyn.com>,
        Pavel Emelyanov <xemul@...tuozzo.com>,
        Cyrill Gorcunov <gorcunov@...nvz.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Willy Tarreau <w@....eu>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        Linux API <linux-api@...r.kernel.org>,
        Linux Containers <containers@...ts.linux-foundation.org>,
        Michael Kerrisk <mtk.manpages@...il.com>
Subject: Re: [PATCH 7/7] signal: Remove kernel interal si_code magic

On Tue, Jul 18, 2017 at 7:06 AM, Eric W. Biederman
<ebiederm@...ssion.com> wrote:
> struct siginfo is a union and the kernel since 2.4 has been hiding a union
> tag in the high 16bits of si_code using the values:
> __SI_KILL
> __SI_TIMER
> __SI_POLL
> __SI_FAULT
> __SI_CHLD
> __SI_RT
> __SI_MESGQ
> __SI_SYS
>
> While this looks plausible on the surface, in practice this situation has
> not worked well.

So on the whole I think we just need to do this, but the part I really
hate about this series is still this the siginfo_layout() part.

I can well believe that it is needed for the compat case. siginfo is a
piece of crap crazy type, and re-ordering fields for compat is
something we are always going to have to do.

But for the native case, the *only* reason we do not just copy the
siginfo as-is seems to be that it's just too big, due to other bad
design decisions in siginfo ("let's make sure it's big enough by
allocating 512 bytes for it).

And afaik, absolutely nobody uses more than about 36 bytes of that
512-byte _sifields union (and that one use is SIGILL with three
pointers and three integers and some padding.

So why don't we just say "screw this idiotic layout crap, and just
unconditionally copy that much smaller maximum of bytes"?

Leave that layout thing purely for compat handling.

Yes, yes, there's a couple of small gotchas's:

 - "_sys_private" for posix timers, and it would have to be moved to
the end of the structure so that it doesn't get copied.

 - make sure those 36 bytes are cleared when allocating the siginfo
(this should be trivial) so that we don't leak any other memory.

But on the whole, it looks pretty straightforward to just get rid of
those stupid layout things, and make them purely about compat stuff.

Please?

The si_code stuff clearly needs to be done regardless, so much of this
patch series looks good to me.  But if we're doign this cleanup, can't
we please go that one extra step and get rid of the crazy "let's treat
the union as different types", and just treat it as a largely opaque
thing.

Pretty please?

                  Linus