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

3 messages, 3 authors, 2021-05-16 · open the first message on its own page

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

From: Eric W. Biederman <hidden>
Date: 2021-05-14 04:55:12

Linus,

Please pull the for-v5.13-rc2 branch from the git tree:

  git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git for-v5.13-rc2

  HEAD: addd6821190ebf1e9fece0b7848db667fd280e2e signalfd: Remove SIL_FAULT_PERF_EVENT fields from signalfd_siginfo

During the merge window an issue with si_perf and the siginfo ABI came
up.  The alpha and sparc siginfo structure layout had changed with the
addition of SIGTRAP TRAP_PERF and the new field si_perf.

The reason only alpha and sparc were affected is that they are the
only architectures that use si_trapno.

Looking deeper it was discovered that si_trapno is used for only
a few select signals on alpha and sparc, and that none of the
other _sigfault fields past si_addr are used at all.  Which means
technically no regression on alpha and sparc.

While the alignment concerns might be dismissed the abuse of
si_errno by SIGTRAP TRAP_PERF does have the potential to cause
regressions in existing userspace.

While we still have time before userspace starts using and depending on
the new definition siginfo for SIGTRAP TRAP_PERF this set of changes
cleans up siginfo_t.

- The si_trapno field is demoted from magic alpha and sparc status and
  made an ordinary union member of the _sigfault member of siginfo_t.
  Without moving it of course.

- si_perf is replaced with si_perf_data and si_perf_type ending the
  abuse of si_errno.

- BUILD_BUG_ONs are added and various helpers are modified to
  accommodate this change.

- Unnecessary additions to signalfd_siginfo are removed.

v3: https://lkml.kernel.org/r/m1tuni8ano.fsf_-_@fess.ebiederm.org
v2: https://lkml.kernel.org/r/m14kfjh8et.fsf_-_@fess.ebiederm.org
v1: https://lkml.kernel.org/r/m1zgxfs7zq.fsf_-_@fess.ebiederm.org

You might notice a recent rebase.  This code has been sitting in
linux-next as  ef566ba2d7d9 ("signal: Remove the last few si_perf
references").  Which results in the exact same code as the branch
I am sending you but the commits differ to keep git bisect working.

The difference is that I squashed a fix for a mips BUILD_BUG_ON about
si_perf into the commit that replaces si_perf with si_perf_data and
si_perf_type.  This keeps the kernel building on all architectures for
all commits keeping git-bisect working for everyone.

Eric W. Biederman (9):
      signal: Verify the alignment and size of siginfo_t
      siginfo: Move si_trapno inside the union inside _si_fault
      signal: Implement SIL_FAULT_TRAPNO
      signal: Use dedicated helpers to send signals with si_trapno set
      signal: Remove __ARCH_SI_TRAPNO
      signal: Rename SIL_PERF_EVENT SIL_FAULT_PERF_EVENT for consistency
      signal: Factor force_sig_perf out of perf_sigtrap
      signal: Deliver all of the siginfo perf data in _perf
      signalfd: Remove SIL_FAULT_PERF_EVENT fields from signalfd_siginfo

Marco Elver (3):
      sparc64: Add compile-time asserts for siginfo_t offsets
      arm: Add compile-time asserts for siginfo_t offsets
      arm64: Add compile-time asserts for siginfo_t offsets

 arch/alpha/include/uapi/asm/siginfo.h              |   2 -
 arch/alpha/kernel/osf_sys.c                        |   2 +-
 arch/alpha/kernel/signal.c                         |   4 +-
 arch/alpha/kernel/traps.c                          |  24 ++---
 arch/alpha/mm/fault.c                              |   4 +-
 arch/arm/kernel/signal.c                           |  39 +++++++
 arch/arm64/kernel/signal.c                         |  39 +++++++
 arch/arm64/kernel/signal32.c                       |  39 +++++++
 arch/m68k/kernel/signal.c                          |   3 +-
 arch/mips/include/uapi/asm/siginfo.h               |   2 -
 arch/sparc/include/uapi/asm/siginfo.h              |   3 -
 arch/sparc/kernel/process_64.c                     |   2 +-
 arch/sparc/kernel/signal32.c                       |  37 +++++++
 arch/sparc/kernel/signal_64.c                      |  36 +++++++
 arch/sparc/kernel/sys_sparc_32.c                   |   2 +-
 arch/sparc/kernel/sys_sparc_64.c                   |   2 +-
 arch/sparc/kernel/traps_32.c                       |  22 ++--
 arch/sparc/kernel/traps_64.c                       |  44 ++++----
 arch/sparc/kernel/unaligned_32.c                   |   2 +-
 arch/sparc/mm/fault_32.c                           |   2 +-
 arch/sparc/mm/fault_64.c                           |   2 +-
 arch/x86/kernel/signal_compat.c                    |  15 ++-
 fs/signalfd.c                                      |  23 ++---
 include/linux/compat.h                             |  10 +-
 include/linux/sched/signal.h                       |  13 +--
 include/linux/signal.h                             |   3 +-
 include/uapi/asm-generic/siginfo.h                 |  20 ++--
 include/uapi/linux/perf_event.h                    |   2 +-
 include/uapi/linux/signalfd.h                      |   4 +-
 kernel/events/core.c                               |  11 +-
 kernel/signal.c                                    | 113 +++++++++++++--------
 .../selftests/perf_events/sigtrap_threads.c        |  14 +--
 32 files changed, 377 insertions(+), 163 deletions(-)

Eric

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

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2021-05-14 19:14:30

On Thu, May 13, 2021 at 9:55 PM Eric W. Biederman [off-list ref] 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.

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.

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.

             Linus

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

From: Ingo Molnar <mingo@kernel.org>
Date: 2021-05-16 07:40:46

* Eric W. Biederman [off-list ref] wrote:
Looking deeper it was discovered that si_trapno is used for only
a few select signals on alpha and sparc, and that none of the
other _sigfault fields past si_addr are used at all.  Which means
technically no regression on alpha and sparc.
If there's no functional regression on any platform, could much of this 
wait until v5.14, or do we want some of these cleanups right now?

The fixes seem to be for long-existing bugs, not fresh regressions, AFAICS. 
The asserts & cleanups are useful, but not regression fixes.

I.e. this is a bit scary:
 32 files changed, 377 insertions(+), 163 deletions(-)
at -rc2 time.

Thanks,

	Ingo
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help