[PATCH v3 05/12] siginfo: Move si_trapno inside the union inside _si_fault

From: Eric W. Beiderman
Date: Wed May 05 2021 - 10:11:34 EST

Next message: Eric W. Beiderman: "[PATCH v3 06/12] signal: Implement SIL_FAULT_TRAPNO"
Previous message: Eric W. Beiderman: "[PATCH v3 04/12] signal: Verify the alignment and size of siginfo_t"
In reply to: Marco Elver: "Re: [PATCH v3 04/12] signal: Verify the alignment and size of siginfo_t"
Next in thread: Eric W. Beiderman: "[PATCH v3 06/12] signal: Implement SIL_FAULT_TRAPNO"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

From: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>

It turns out that linux uses si_trapno very sparingly, and as such it
can be considered extra information for a very narrow selection of
signals, rather than information that is present with every fault
reported in siginfo.

As such move si_trapno inside the union inside of _si_fault. This
results in no change in placement, and makes it eaiser
to extend _si_fault in the future as this reduces the number of
special cases. In particular with si_trapno included in the union it
is no longer a concern that the union must be pointer alligned on most
architectures because the union followes immediately after si_addr
which is a pointer.

This change results in a difference in siginfo field placement on
sparc and alpha for the fields si_addr_lsb, si_lower, si_upper,
si_pkey, and si_perf. These architectures do not implement the
signals that would use si_addr_lsb, si_lower, si_upper, si_pkey, and
si_perf. Further these architecture have not yet implemented the
userspace that would use si_perf.

The point of this change is in fact to correct these placement issues
before sparc or alpha grow userspace that cares. This change was
discussed[1] and the agreement is that this change is currently safe.

[1]: https://lkml.kernel.org/r/CAK8P3a0+uKYwL1NhY6Hvtieghba2hKYGD6hcKx5n8=4Gtt+pHA@xxxxxxxxxxxxxx
Acked-by: Marco Elver <elver@xxxxxxxxxx>
v1: https://lkml.kernel.org/r/m1tunns7yf.fsf_-_@xxxxxxxxxxxxxxxxx
Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
---
arch/sparc/kernel/signal32.c | 10 +++++-----
arch/sparc/kernel/signal_64.c | 10 +++++-----
arch/x86/kernel/signal_compat.c | 3 +++
include/linux/compat.h | 5 ++---
include/uapi/asm-generic/siginfo.h | 7 ++-----
kernel/signal.c | 1 +
6 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/arch/sparc/kernel/signal32.c b/arch/sparc/kernel/signal32.c
index 32b977f253e3..5573722e34ad 100644
--- a/arch/sparc/kernel/signal32.c
+++ b/arch/sparc/kernel/signal32.c
@@ -774,10 +774,10 @@ static_assert(offsetof(compat_siginfo_t, si_int) == 0x14);
static_assert(offsetof(compat_siginfo_t, si_ptr) == 0x14);
static_assert(offsetof(compat_siginfo_t, si_addr) == 0x0c);
static_assert(offsetof(compat_siginfo_t, si_trapno) == 0x10);
-static_assert(offsetof(compat_siginfo_t, si_addr_lsb) == 0x14);
-static_assert(offsetof(compat_siginfo_t, si_lower) == 0x18);
-static_assert(offsetof(compat_siginfo_t, si_upper) == 0x1c);
-static_assert(offsetof(compat_siginfo_t, si_pkey) == 0x18);
-static_assert(offsetof(compat_siginfo_t, si_perf) == 0x14);
+static_assert(offsetof(compat_siginfo_t, si_addr_lsb) == 0x10);
+static_assert(offsetof(compat_siginfo_t, si_lower) == 0x14);
+static_assert(offsetof(compat_siginfo_t, si_upper) == 0x18);
+static_assert(offsetof(compat_siginfo_t, si_pkey) == 0x14);
+static_assert(offsetof(compat_siginfo_t, si_perf) == 0x10);
static_assert(offsetof(compat_siginfo_t, si_band) == 0x0c);
static_assert(offsetof(compat_siginfo_t, si_fd) == 0x10);
diff --git a/arch/sparc/kernel/signal_64.c b/arch/sparc/kernel/signal_64.c
index e9dda9db156c..a69a78984c36 100644
--- a/arch/sparc/kernel/signal_64.c
+++ b/arch/sparc/kernel/signal_64.c
@@ -584,10 +584,10 @@ static_assert(offsetof(siginfo_t, si_int) == 0x18);
static_assert(offsetof(siginfo_t, si_ptr) == 0x18);
static_assert(offsetof(siginfo_t, si_addr) == 0x10);
static_assert(offsetof(siginfo_t, si_trapno) == 0x18);
-static_assert(offsetof(siginfo_t, si_addr_lsb) == 0x20);
-static_assert(offsetof(siginfo_t, si_lower) == 0x28);
-static_assert(offsetof(siginfo_t, si_upper) == 0x30);
-static_assert(offsetof(siginfo_t, si_pkey) == 0x28);
-static_assert(offsetof(siginfo_t, si_perf) == 0x20);
+static_assert(offsetof(siginfo_t, si_addr_lsb) == 0x18);
+static_assert(offsetof(siginfo_t, si_lower) == 0x20);
+static_assert(offsetof(siginfo_t, si_upper) == 0x28);
+static_assert(offsetof(siginfo_t, si_pkey) == 0x20);
+static_assert(offsetof(siginfo_t, si_perf) == 0x18);
static_assert(offsetof(siginfo_t, si_band) == 0x10);
static_assert(offsetof(siginfo_t, si_fd) == 0x14);
diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c
index e735bc129331..c9601f092a1e 100644
--- a/arch/x86/kernel/signal_compat.c
+++ b/arch/x86/kernel/signal_compat.c
@@ -133,6 +133,9 @@ static inline void signal_compat_build_tests(void)
BUILD_BUG_ON(offsetof(siginfo_t, si_addr) != 0x10);
BUILD_BUG_ON(offsetof(compat_siginfo_t, si_addr) != 0x0C);

+ BUILD_BUG_ON(offsetof(siginfo_t, si_trapno) != 0x18);
+ BUILD_BUG_ON(offsetof(compat_siginfo_t, si_trapno) != 0x10);
+
BUILD_BUG_ON(offsetof(siginfo_t, si_addr_lsb) != 0x18);
BUILD_BUG_ON(offsetof(compat_siginfo_t, si_addr_lsb) != 0x10);

diff --git a/include/linux/compat.h b/include/linux/compat.h
index f0d2dd35d408..6af7bef15e94 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -214,12 +214,11 @@ typedef struct compat_siginfo {
/* SIGILL, SIGFPE, SIGSEGV, SIGBUS, SIGTRAP, SIGEMT */
struct {
compat_uptr_t _addr; /* faulting insn/memory ref. */
-#ifdef __ARCH_SI_TRAPNO
- int _trapno; /* TRAP # which caused the signal */
-#endif
#define __COMPAT_ADDR_BND_PKEY_PAD (__alignof__(compat_uptr_t) < sizeof(short) ? \
sizeof(short) : __alignof__(compat_uptr_t))
union {
+ /* used on alpha and sparc */
+ int _trapno; /* TRAP # which caused the signal */
/*
* used when si_code=BUS_MCEERR_AR or
* used when si_code=BUS_MCEERR_AO
diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index 91c80d0c10c5..3503282021aa 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -68,9 +68,6 @@ union __sifields {
/* SIGILL, SIGFPE, SIGSEGV, SIGBUS, SIGTRAP, SIGEMT */
struct {
void __user *_addr; /* faulting insn/memory ref. */
-#ifdef __ARCH_SI_TRAPNO
- int _trapno; /* TRAP # which caused the signal */
-#endif
#ifdef __ia64__
int _imm; /* immediate value for "break" */
unsigned int _flags; /* see ia64 si_flags */
@@ -80,6 +77,8 @@ union __sifields {
#define __ADDR_BND_PKEY_PAD (__alignof__(void *) < sizeof(short) ? \
sizeof(short) : __alignof__(void *))
union {
+ /* used on alpha and sparc */
+ int _trapno; /* TRAP # which caused the signal */
/*
* used when si_code=BUS_MCEERR_AR or
* used when si_code=BUS_MCEERR_AO
@@ -155,9 +154,7 @@ typedef struct siginfo {
#define si_int _sifields._rt._sigval.sival_int
#define si_ptr _sifields._rt._sigval.sival_ptr
#define si_addr _sifields._sigfault._addr
-#ifdef __ARCH_SI_TRAPNO
#define si_trapno _sifields._sigfault._trapno
-#endif
#define si_addr_lsb _sifields._sigfault._addr_lsb
#define si_lower _sifields._sigfault._addr_bnd._lower
#define si_upper _sifields._sigfault._addr_bnd._upper
diff --git a/kernel/signal.c b/kernel/signal.c
index c3017aa8024a..65888aec65a0 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -4607,6 +4607,7 @@ static inline void siginfo_buildtime_checks(void)

/* sigfault */
CHECK_OFFSET(si_addr);
+ CHECK_OFFSET(si_trapno);
CHECK_OFFSET(si_addr_lsb);
CHECK_OFFSET(si_lower);
CHECK_OFFSET(si_upper);
--
2.30.1

Next message: Eric W. Beiderman: "[PATCH v3 06/12] signal: Implement SIL_FAULT_TRAPNO"
Previous message: Eric W. Beiderman: "[PATCH v3 04/12] signal: Verify the alignment and size of siginfo_t"
In reply to: Marco Elver: "Re: [PATCH v3 04/12] signal: Verify the alignment and size of siginfo_t"
Next in thread: Eric W. Beiderman: "[PATCH v3 06/12] signal: Implement SIL_FAULT_TRAPNO"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]