linux-kernel - Re: [GIT PULL] siginfo: ABI fixes for v5.13-rc2

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]

Message-ID: <m14kf5aufb.fsf@fess.ebiederm.org>
Date:   Fri, 14 May 2021 16:15:36 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Arnd Bergmann <arnd@...db.de>, Florian Weimer <fweimer@...hat.com>,
        "David S. Miller" <davem@...emloft.net>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Collingbourne <pcc@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Alexander Potapenko <glider@...gle.com>,
        sparclinux <sparclinux@...r.kernel.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux API <linux-api@...r.kernel.org>,
        kasan-dev <kasan-dev@...glegroups.com>,
        Marco Elver <elver@...gle.com>
Subject: Re: [GIT PULL] siginfo: ABI fixes for v5.13-rc2

Linus Torvalds <torvalds@...ux-foundation.org> writes:

> On Thu, May 13, 2021 at 9:55 PM Eric W. Biederman <ebiederm@...ssion.com> wrote:
>>
>> Please pull the for-v5.13-rc2 branch from the git tree:
>
> I really don't like this tree.
>
> The immediate cause for "no" is the silly
>
>  #if IS_ENABLED(CONFIG_SPARC)
>
> and
>
>  #if IS_ENABLED(CONFIG_ALPHA)
>
> code in kernel/signal.c. It has absolutely zero business being there,
> when those architectures have a perfectly fine arch/*/kernel/signal.c
> file where that code would make much more sense *WITHOUT* any odd
> preprocessor games.

The code is generic it just happens those functions are only used on
sparc and alpha.  Further I really want to make filling out siginfo_t
happen in dedicated functions as much as possible in kernel/signal.c.
The probably of getting it wrong without a helper functions is very
strong.  As the code I am fixing demonstrates.

The IS_ENABLED(arch) is mostly there so we can delete the code if/when
the architectures are retired in another decade or so.

> But there are other oddities too, like the new
>
>     send_sig_fault_trapno(SIGFPE, si_code, (void __user *) regs->pc,
> 0, current);
>
> in the alpha code, which fundamentally seems bogus: using
> send_sig_fault_trapno() with a '0' for trapno seems entirely
> incorrect, since the *ONLY* point of that function is to set si_trapno
> to something non-zero.
>
> So it would seem that a plain send_sig_fault() without that 0 would be
> the right thing to do.

As it happens the floating point emulation code on alpha is inconsistent
with the non floating point emulation code.  When using real floating
point hardware SIGFPE on alpha always set si_trapno.  The floating point
emulation code does not look like it has ever set si_trapno.

I continued to used send_sig_fault_trapno to point out that
inconsistency.

If alpha floating point emulation was in active use I expect we would
care enough to put something other than 0 in there.

> This also mixes in a lot of other stuff than just the fixes. Which
> would have been ok during the merge window, but I'm definitely not
> happy about it now.

If the breakage that came with SIGTRAP TRAP_PERF had not been discovered
during the merge window I would not be sending this now.  It took a
little time to dig to the bottom, then the code needed just a little
extra time to sit, so there were not surprises.

As for mixing things, I am not quite certain what you are referring to.
All of the changes relate to keeping people from shooting themselves
in the foot with when using siginfo.

The most noise comes from send_sig_fault vs send_sig_fault_trapno, and
force_sig_fault vs force_sig_fault_trapno.  That is fundamental to the
siginfo fix as it is there to ensure that is safe to treat si_trapno
as an ordinary _sigfault union member.  Which in turns makes alpha
and sparc no longer special with respect to _sigfault, just a little
eccentric.

I will concede that renaming SIL_PERF_EVENT to SIL_FAULT_PERF_EVENT is
unnecessary, but it certainly makes it clear that we are dealing
with _sigfault and not some other part of siginfo_t.

Eric