siginfo_t ABI break on sparc64 from si_addr_lsb move 3y ago

5 views
Skip to first unread message

Marco Elver

unread,
Apr 29, 2021, 3:48:20 AMApr 29
to Eric W. Biederman, Florian Weimer, David S. Miller, Arnd Bergmann, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparc...@vger.kernel.org, linux...@vger.kernel.org, linux-...@vger.kernel.org, linu...@vger.kernel.org, kasa...@googlegroups.com
Hello, Eric,

By inspecting the logs I've seen that about 3 years ago there had been a
number of siginfo_t cleanups. This included moving si_addr_lsb:

b68a68d3dcc1 ("signal: Move addr_lsb into the _sigfault union for clarity")
859d880cf544 ("signal: Correct the offset of si_pkey in struct siginfo")
8420f71943ae ("signal: Correct the offset of si_pkey and si_lower in struct siginfo on m68k")

In an ideal world, we could just have si_addr + the union in _sigfault,
but it seems there are more corner cases. :-/

The reason I've stumbled upon this is that I wanted to add the just
merged si_perf [1] field to glibc. But what I noticed is that glibc's
definition and ours are vastly different around si_addr_lsb, si_lower,
si_upper, and si_pkey.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=42dec9a936e7696bea1f27d3c5a0068cd9aa95fd

In our current definition of siginfo_t, si_addr_lsb is placed into the
same union as si_lower, si_upper, and si_pkey (and now si_perf). From
the logs I see that si_lower, si_upper, and si_pkey are padded because
si_addr_lsb used to be outside the union, which goes back to
"signal: Move addr_lsb into the _sigfault union for clarity".

Since then, si_addr_lsb must also be pointer-aligned, because the union
containing it must be pointer-aligned (because si_upper, si_lower). On
all architectures where si_addr_lsb is right after si_addr, this is
perfectly fine, because si_addr itself is a pointer...

... except for the anomaly that are 64-bit architectures that define
__ARCH_SI_TRAPNO and want that 'int si_trapno'. Like, for example
sparc64, which means siginfo_t's ABI has been subtly broken on sparc64
since v4.16.

The following static asserts illustrate this:

--- a/arch/sparc/kernel/signal_64.c
+++ b/arch/sparc/kernel/signal_64.c
@@ -556,3 +556,37 @@ void do_notify_resume(struct pt_regs *regs, unsigned long orig_i0, unsigned long
user_enter();
}

+static_assert(offsetof(siginfo_t, si_signo) == 0);
+static_assert(offsetof(siginfo_t, si_errno) == 4);
+static_assert(offsetof(siginfo_t, si_code) == 8);
+static_assert(offsetof(siginfo_t, si_pid) == 16);
+static_assert(offsetof(siginfo_t, si_uid) == 20);
+static_assert(offsetof(siginfo_t, si_tid) == 16);
+static_assert(offsetof(siginfo_t, si_overrun) == 20);
+static_assert(offsetof(siginfo_t, si_status) == 24);
+static_assert(offsetof(siginfo_t, si_utime) == 32);
+static_assert(offsetof(siginfo_t, si_stime) == 40);
+static_assert(offsetof(siginfo_t, si_value) == 24);
+static_assert(offsetof(siginfo_t, si_int) == 24);
+static_assert(offsetof(siginfo_t, si_ptr) == 24);
+static_assert(offsetof(siginfo_t, si_addr) == 16);
+static_assert(offsetof(siginfo_t, si_trapno) == 24);
+#if 1 /* Correct offsets, obtained from v4.14 */
+static_assert(offsetof(siginfo_t, si_addr_lsb) == 28);
+static_assert(offsetof(siginfo_t, si_lower) == 32);
+static_assert(offsetof(siginfo_t, si_upper) == 40);
+static_assert(offsetof(siginfo_t, si_pkey) == 32);
+#else /* Current offsets, as of v4.16 */
+static_assert(offsetof(siginfo_t, si_addr_lsb) == 32);
+static_assert(offsetof(siginfo_t, si_lower) == 40);
+static_assert(offsetof(siginfo_t, si_upper) == 48);
+static_assert(offsetof(siginfo_t, si_pkey) == 40);
+#endif
+static_assert(offsetof(siginfo_t, si_band) == 16);
+static_assert(offsetof(siginfo_t, si_fd) == 20);

---

Granted, nobody seems to have noticed because I don't even know if these
fields have use on sparc64. But I don't yet see this as justification to
leave things as-is...

The collateral damage of this, and the acute problem that I'm having is
defining si_perf in a sort-of readable and portable way in siginfo_t
definitions that live outside the kernel, where sparc64 does not yet
have broken si_addr_lsb. And the same difficulty applies to the kernel
if we want to unbreak sparc64, while not wanting to move si_perf for
other architectures.

There are 2 options I see to solve this:

1. Make things simple again. We could just revert the change moving
si_addr_lsb into the union, and sadly accept we'll have to live with
that legacy "design" mistake. (si_perf stays in the union, but will
unfortunately change its offset for all architectures... this one-off
move might be ok because it's new.)

2. Add special cases to retain si_addr_lsb in the union on architectures
that do not have __ARCH_SI_TRAPNO (the majority). I have added a
draft patch that would do this below (with some refactoring so that
it remains sort-of readable), as an experiment to see how complicated
this gets.

Option (1) means we'll forever be wasting the space where si_addr_lsb
lives (including the padding). It'd also mean we'd move si_perf for
_all_ architectures -- this might be acceptable, given there is no
stable release with it yet -- the fix just needs to be merged before the
release of v5.13! It is the simpler option though -- and I don't know if
we need all this complexity.

Option (2) perhaps results in better space utilization. Maybe that's
better long-term if we worry about space in some rather distant future
-- where we need those 8 bytes on 64-bit architectures to not exceed 128
bytes. This option, however, doesn't just make us carry this complexity
forever, but also forces it onto everyone else, like glibc and other
libcs (including those in other languages with C FFIs) which have their
own definition of siginfo_t.

Which option do you prefer? Are there better options?

Many thanks,
-- Marco

------ >8 ------

Option #2 draft:

diff --git a/arch/sparc/kernel/signal_64.c b/arch/sparc/kernel/signal_64.c
index a0eec62c825d..150ee27b1423 100644
--- a/arch/sparc/kernel/signal_64.c
+++ b/arch/sparc/kernel/signal_64.c
@@ -556,3 +556,37 @@ void do_notify_resume(struct pt_regs *regs, unsigned long orig_i0, unsigned long
user_enter();
}

+/*
+ * Compile-time assertions for siginfo_t offsets. Unlike other architectures,
+ * sparc64 is special, because it requires si_trapno (int), and the following
+ * si_addr_lsb (short) need not be word aligned. Accidental changes around the
+ * offset of si_addr_lsb and the following fields would only be caught here.
+ */
+static_assert(offsetof(siginfo_t, si_signo) == 0);
+static_assert(offsetof(siginfo_t, si_errno) == 4);
+static_assert(offsetof(siginfo_t, si_code) == 8);
+static_assert(offsetof(siginfo_t, si_pid) == 16);
+static_assert(offsetof(siginfo_t, si_uid) == 20);
+static_assert(offsetof(siginfo_t, si_tid) == 16);
+static_assert(offsetof(siginfo_t, si_overrun) == 20);
+static_assert(offsetof(siginfo_t, si_status) == 24);
+static_assert(offsetof(siginfo_t, si_utime) == 32);
+static_assert(offsetof(siginfo_t, si_stime) == 40);
+static_assert(offsetof(siginfo_t, si_value) == 24);
+static_assert(offsetof(siginfo_t, si_int) == 24);
+static_assert(offsetof(siginfo_t, si_ptr) == 24);
+static_assert(offsetof(siginfo_t, si_addr) == 16);
+static_assert(offsetof(siginfo_t, si_trapno) == 24);
+#if 1 /* Correct offsets, obtained from v4.14 */
+static_assert(offsetof(siginfo_t, si_addr_lsb) == 28);
+static_assert(offsetof(siginfo_t, si_lower) == 32);
+static_assert(offsetof(siginfo_t, si_upper) == 40);
+static_assert(offsetof(siginfo_t, si_pkey) == 32);
+#else /* Current offsets, as of v4.16 */
+static_assert(offsetof(siginfo_t, si_addr_lsb) == 32);
+static_assert(offsetof(siginfo_t, si_lower) == 40);
+static_assert(offsetof(siginfo_t, si_upper) == 48);
+static_assert(offsetof(siginfo_t, si_pkey) == 40);
+#endif
+static_assert(offsetof(siginfo_t, si_band) == 16);
+static_assert(offsetof(siginfo_t, si_fd) == 20);
diff --git a/include/linux/compat.h b/include/linux/compat.h
index f0d2dd35d408..5ea9f9c748dd 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -158,6 +158,31 @@ typedef union compat_sigval {
compat_uptr_t sival_ptr;
} compat_sigval_t;

+struct __compat_sigfault_addin {
+#ifdef __ARCH_SI_TRAPNO
+ int _trapno; /* TRAP # which caused the signal */
+#endif
+ /*
+ * used when si_code=BUS_MCEERR_AR or
+ * used when si_code=BUS_MCEERR_AO
+ */
+ short int _addr_lsb; /* Valid LSB of the reported address. */
+
+/* See include/asm-generic/uapi/siginfo.h */
+#ifdef __ARCH_SI_TRAPNO
+# define __COMPAT_SIGFAULT_ADDIN_FIXED struct __compat_sigfault_addin _addin;
+# define __COMPAT_SIGFAULT_ADDIN_UNION
+# define __COMPAT_SIGFAULT_LEGACY_UNION_PAD
+#else
+# define __COMPAT_SIGFAULT_ADDIN_FIXED
+# define __COMPAT_SIGFAULT_ADDIN_UNION struct __compat_sigfault_addin _addin;
+# define __COMPAT_SIGFAULT_LEGACY_UNION_PAD \
+ char _unused[__alignof__(compat_uptr_t) < sizeof(short) ? \
+ sizeof(short) : \
+ __alignof__(compat_uptr_t)];
+#endif
+};
+
typedef struct compat_siginfo {
int si_signo;
#ifndef __ARCH_HAS_SWAPPED_SIGINFO
@@ -214,26 +239,18 @@ 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))
+ __COMPAT_SIGFAULT_ADDIN_FIXED
union {
- /*
- * used when si_code=BUS_MCEERR_AR or
- * used when si_code=BUS_MCEERR_AO
- */
- short int _addr_lsb; /* Valid LSB of the reported address. */
+ __COMPAT_SIGFAULT_ADDIN_UNION
/* used when si_code=SEGV_BNDERR */
struct {
- char _dummy_bnd[__COMPAT_ADDR_BND_PKEY_PAD];
+ __COMPAT_SIGFAULT_LEGACY_UNION_PAD
compat_uptr_t _lower;
compat_uptr_t _upper;
} _addr_bnd;
/* used when si_code=SEGV_PKUERR */
struct {
- char _dummy_pkey[__COMPAT_ADDR_BND_PKEY_PAD];
+ __COMPAT_SIGFAULT_LEGACY_UNION_PAD
u32 _pkey;
} _addr_pkey;
/* used when si_code=TRAP_PERF */
diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index 03d6f6d2c1fe..f1c1a0300ac8 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -29,6 +29,45 @@ typedef union sigval {
#define __ARCH_SI_ATTRIBUTES
#endif

+/*
+ * The _sigfault portion of __sifields after si_addr varies depending on
+ * architecture; capture these rules here.
+ */
+struct __sifields_sigfault_addin {
+#ifdef __ARCH_SI_TRAPNO
+ int _trapno; /* TRAP # which caused the signal */
+#endif
+ /*
+ * used when si_code=BUS_MCEERR_AR or
+ * used when si_code=BUS_MCEERR_AO
+ */
+ short _addr_lsb; /* LSB of the reported address */
+
+#if defined(__ARCH_SI_TRAPNO)
+/*
+ * If we have si_trapno between si_addr and si_addr_lsb, we cannot safely move
+ * it inside the union due to alignment of si_trapno+si_addr_lsb vs. the union.
+ */
+# define __SI_SIGFAULT_ADDIN_FIXED struct __sifields_sigfault_addin _addin;
+# define __SI_SIGFAULT_ADDIN_UNION
+# define __SI_SIGFAULT_LEGACY_UNION_PAD
+#else
+/*
+ * Safe to move si_addr_lsb inside the union. We will benefit from better space
+ * usage for new fields added to the union.
+ *
+ * Fields that were added after si_addr_lsb, before it become part of the union,
+ * require padding to retain the ABI. New fields do not require padding.
+ */
+# define __SI_SIGFAULT_ADDIN_FIXED
+# define __SI_SIGFAULT_ADDIN_UNION struct __sifields_sigfault_addin _addin;
+# define __SI_SIGFAULT_LEGACY_UNION_PAD \
+ char _unused[__alignof__(void *) < sizeof(short) ? \
+ sizeof(short) : \
+ __alignof__(void *)];
+#endif
+};
+
union __sifields {
/* kill() */
struct {
@@ -63,32 +102,23 @@ 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 */
unsigned long _isr; /* isr */
#endif
-
-#define __ADDR_BND_PKEY_PAD (__alignof__(void *) < sizeof(short) ? \
- sizeof(short) : __alignof__(void *))
+ __SI_SIGFAULT_ADDIN_FIXED
union {
- /*
- * used when si_code=BUS_MCEERR_AR or
- * used when si_code=BUS_MCEERR_AO
- */
- short _addr_lsb; /* LSB of the reported address */
+ __SI_SIGFAULT_ADDIN_UNION
/* used when si_code=SEGV_BNDERR */
struct {
- char _dummy_bnd[__ADDR_BND_PKEY_PAD];
+ __SI_SIGFAULT_LEGACY_UNION_PAD
void __user *_lower;
void __user *_upper;
} _addr_bnd;
/* used when si_code=SEGV_PKUERR */
struct {
- char _dummy_pkey[__ADDR_BND_PKEY_PAD];
+ __SI_SIGFAULT_LEGACY_UNION_PAD
__u32 _pkey;
} _addr_pkey;
/* used when si_code=TRAP_PERF */
@@ -151,9 +181,9 @@ typedef struct siginfo {
#define si_ptr _sifields._rt._sigval.sival_ptr
#define si_addr _sifields._sigfault._addr
#ifdef __ARCH_SI_TRAPNO
-#define si_trapno _sifields._sigfault._trapno
+#define si_trapno _sifields._sigfault._addin._trapno
#endif
-#define si_addr_lsb _sifields._sigfault._addr_lsb
+#define si_addr_lsb _sifields._sigfault._addin._addr_lsb
#define si_lower _sifields._sigfault._addr_bnd._lower
#define si_upper _sifields._sigfault._addr_bnd._upper
#define si_pkey _sifields._sigfault._addr_pkey._pkey

Eric W. Biederman

unread,
Apr 29, 2021, 1:24:05 PMApr 29
to Marco Elver, Florian Weimer, David S. Miller, Arnd Bergmann, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparc...@vger.kernel.org, linux...@vger.kernel.org, linux-...@vger.kernel.org, linu...@vger.kernel.org, kasa...@googlegroups.com
> Which option do you prefer? Are there better options?

Personally the most important thing to have is a single definition
shared by all architectures so that we consolidate testing.

A little piece of me cries a little whenever I see how badly we
implemented the POSIX design. As specified by POSIX the fields can be
place in siginfo such that 32bit and 64bit share a common definition.
Unfortunately we did not addpadding after si_addr on 32bit to
accommodate a 64bit si_addr.

I find it unfortunate that we are adding yet another definition that
requires translation between 32bit and 64bit, but I am glad
that at least the translation is not architecture specific. That common
definition is what has allowed this potential issue to be caught
and that makes me very happy to see.

Let's go with Option 3.

Confirm BUS_MCEERR_AR, BUS_MCEERR_AO, SEGV_BNDERR, SEGV_PKUERR are not
in use on any architecture that defines __ARCH_SI_TRAPNO, and then fixup
the userspace definitions of these fields.

To the kernel I would add some BUILD_BUG_ON's to whatever the best
maintained architecture (sparc64?) that implements __ARCH_SI_TRAPNO just
to confirm we don't create future regressions by accident.

I did a quick search and the architectures that define __ARCH_SI_TRAPNO
are sparc, mips, and alpha. All have 64bit implementations. A further
quick search shows that none of those architectures have faults that
use BUS_MCEERR_AR, BUS_MCEERR_AO, SEGV_BNDERR, SEGV_PKUERR, nor do
they appear to use mm/memory-failure.c

So it doesn't look like we have an ABI regression to fix.

Eric

Marco Elver

unread,
Apr 29, 2021, 2:47:01 PMApr 29
to Eric W. Biederman, Florian Weimer, David S. Miller, Arnd Bergmann, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparc...@vger.kernel.org, linux-arch, LKML, linu...@vger.kernel.org, kasan-dev
On Thu, 29 Apr 2021 at 19:24, Eric W. Biederman <ebie...@xmission.com> wrote:
[...]
I think it's even worse than that, see the fun I had with siginfo last
week: https://lkml.kernel.org/r/20210422191823...@google.com
... because of the 3 initial ints and no padding after them, we can't
portably add __u64 fields to siginfo, and are forever forced to have
subtly different behaviour between 32-bit and 64-bit architectures.
:-/

> I find it unfortunate that we are adding yet another definition that
> requires translation between 32bit and 64bit, but I am glad
> that at least the translation is not architecture specific. That common
> definition is what has allowed this potential issue to be caught
> and that makes me very happy to see.
>
> Let's go with Option 3.
>
> Confirm BUS_MCEERR_AR, BUS_MCEERR_AO, SEGV_BNDERR, SEGV_PKUERR are not
> in use on any architecture that defines __ARCH_SI_TRAPNO, and then fixup
> the userspace definitions of these fields.
>
> To the kernel I would add some BUILD_BUG_ON's to whatever the best
> maintained architecture (sparc64?) that implements __ARCH_SI_TRAPNO just
> to confirm we don't create future regressions by accident.
>
> I did a quick search and the architectures that define __ARCH_SI_TRAPNO
> are sparc, mips, and alpha. All have 64bit implementations. A further
> quick search shows that none of those architectures have faults that
> use BUS_MCEERR_AR, BUS_MCEERR_AO, SEGV_BNDERR, SEGV_PKUERR, nor do
> they appear to use mm/memory-failure.c
>
> So it doesn't look like we have an ABI regression to fix.

That sounds fine to me -- my guess was that they're not used on these
architectures, but I just couldn't make that call.

I have patches adding compile-time asserts for sparc64, arm, arm64
ready to go. I'll send them after some more testing.

Thanks,
-- Marco

Arnd Bergmann

unread,
Apr 29, 2021, 4:49:19 PMApr 29
to Eric W. Biederman, Marco Elver, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
On Thu, Apr 29, 2021 at 7:23 PM Eric W. Biederman <ebie...@xmission.com> wrote:

> > Which option do you prefer? Are there better options?
>
> Personally the most important thing to have is a single definition
> shared by all architectures so that we consolidate testing.
>
> A little piece of me cries a little whenever I see how badly we
> implemented the POSIX design. As specified by POSIX the fields can be
> place in siginfo such that 32bit and 64bit share a common definition.
> Unfortunately we did not addpadding after si_addr on 32bit to
> accommodate a 64bit si_addr.
>
> I find it unfortunate that we are adding yet another definition that
> requires translation between 32bit and 64bit, but I am glad
> that at least the translation is not architecture specific. That common
> definition is what has allowed this potential issue to be caught
> and that makes me very happy to see.
>
> Let's go with Option 3.
>
> Confirm BUS_MCEERR_AR, BUS_MCEERR_AO, SEGV_BNDERR, SEGV_PKUERR are not
> in use on any architecture that defines __ARCH_SI_TRAPNO, and then fixup
> the userspace definitions of these fields.
>
> To the kernel I would add some BUILD_BUG_ON's to whatever the best
> maintained architecture (sparc64?) that implements __ARCH_SI_TRAPNO just
> to confirm we don't create future regressions by accident.
>
> I did a quick search and the architectures that define __ARCH_SI_TRAPNO
> are sparc, mips, and alpha. All have 64bit implementations.

I think you (slightly) misread: mips has "#undef __ARCH_SI_TRAPNO", not
"#define __ARCH_SI_TRAPNO". This means it's only sparc and
alpha.

I can see that the alpha instance was added to the kernel during linux-2.5,
but never made it into the glibc or uclibc copy of the struct definition, and
musl doesn't support alpha or sparc. Debian codesearch only turns up
sparc (and BSD) references to si_trapno.

> I did a quick search and the architectures that define __ARCH_SI_TRAPNO
> are sparc, mips, and alpha. All have 64bit implementations. A further
> quick search shows that none of those architectures have faults that
> use BUS_MCEERR_AR, BUS_MCEERR_AO, SEGV_BNDERR, SEGV_PKUERR, nor do
> they appear to use mm/memory-failure.c
>
> So it doesn't look like we have an ABI regression to fix.

Even better!

So if sparc is the only user of _trapno and it uses none of the later
fields in _sigfault, I wonder if we could take even more liberty at
trying to have a slightly saner definition. Can you think of anything that
might break if we put _trapno inside of the union along with _perf
and _addr_lsb?

I suppose in theory sparc64 or alpha might start using the other
fields in the future, and an application might be compiled against
mismatched headers, but that is unlikely and is already broken
with the current headers.

Arnd

Eric W. Biederman

unread,
Apr 30, 2021, 1:08:31 PMApr 30
to Arnd Bergmann, Marco Elver, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
On sparc si_trapno is only set when SIGILL ILL_TRP is set. So we can
limit si_trapno to that combination, and it should not be a problem for
a new signal/si_code pair to use that storage. Precisely because it is
new.

Similarly on alpha si_trapno is only set for:

SIGFPE {FPE_INTOVF, FPE_INTDIV, FPE_FLTOVF, FPE_FLTDIV, FPE_FLTUND,
FPE_FLTINV, FPE_FLTRES, FPE_FLTUNK} and SIGTRAP {TRAP_UNK}.

Placing si_trapno into the union would also make the problem that the
union is pointer aligned a non-problem as then the union immediate
follows a pointer.

I hadn't had a chance to look before but we must deal with this. The
definition of perf_sigtrap in 42dec9a936e7696bea1f27d3c5a0068cd9aa95fd
is broken on sparc, alpha, and ia64 as it bypasses the code in
kernel/signal.c that ensures the si_trapno or the ia64 special fields
are set.

Not to mention that perf_sigtrap appears to abuse si_errno.

The code is only safe if the analysis that says we can move si_trapno
and perhaps the ia64 fields into the union is correct. It looks like
ia64 much more actively uses it's signal extension fields including for
SIGTRAP, so I am not at all certain the generic definition of
perf_sigtrap is safe on ia64.

> I suppose in theory sparc64 or alpha might start using the other
> fields in the future, and an application might be compiled against
> mismatched headers, but that is unlikely and is already broken
> with the current headers.

If we localize the use of si_trapno to just a few special cases on alpha
and sparc I think we don't even need to worry about breaking userspace
on any architecture. It will complicate siginfo_layout, but it is a
complication that reflects reality.

I don't have a clue how any of this affects ia64. Does perf work on
ia64? Does perf work on sparc, and alpha?

If perf works on ia64 we need to take a hard look at what is going on
there as well.

Eric

Marco Elver

unread,
Apr 30, 2021, 3:07:14 PMApr 30
to Eric W. Biederman, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
On Fri, Apr 30, 2021 at 12:08PM -0500, Eric W. Biederman wrote:
> Arnd Bergmann <ar...@arndb.de> writes:
[...]
There are a few other places in the kernel that repurpose si_errno
similarly, e.g. arch/arm64/kernel/ptrace.c, kernel/seccomp.c -- it was
either that or introduce another field or not have it. It is likely we
could do without, but if there are different event types the user would
have to sacrifice a few bits of si_perf to encode the event type, and
I'd rather keep those bits for something else. Thus the decision fell to
use si_errno.

Given it'd be wasted space otherwise, and we define the semantics of
whatever is stored in siginfo on the new signal, it'd be good to keep.

> The code is only safe if the analysis that says we can move si_trapno
> and perhaps the ia64 fields into the union is correct. It looks like
> ia64 much more actively uses it's signal extension fields including for
> SIGTRAP, so I am not at all certain the generic definition of
> perf_sigtrap is safe on ia64.

Trying to understand the requirements of si_trapno myself: safe here
would mean that si_trapno is not required if we fire our SIGTRAP /
TRAP_PERF.

As far as I can tell that is the case -- see below.

> > I suppose in theory sparc64 or alpha might start using the other
> > fields in the future, and an application might be compiled against
> > mismatched headers, but that is unlikely and is already broken
> > with the current headers.
>
> If we localize the use of si_trapno to just a few special cases on alpha
> and sparc I think we don't even need to worry about breaking userspace
> on any architecture. It will complicate siginfo_layout, but it is a
> complication that reflects reality.
>
> I don't have a clue how any of this affects ia64. Does perf work on
> ia64? Does perf work on sparc, and alpha?
>
> If perf works on ia64 we need to take a hard look at what is going on
> there as well.

No perf on ia64, but it seems alpha and sparc have perf:

$ git grep 'select.*HAVE_PERF_EVENTS$' -- arch/
arch/alpha/Kconfig: select HAVE_PERF_EVENTS <--
arch/arc/Kconfig: select HAVE_PERF_EVENTS
arch/arm/Kconfig: select HAVE_PERF_EVENTS
arch/arm64/Kconfig: select HAVE_PERF_EVENTS
arch/csky/Kconfig: select HAVE_PERF_EVENTS
arch/hexagon/Kconfig: select HAVE_PERF_EVENTS
arch/mips/Kconfig: select HAVE_PERF_EVENTS
arch/nds32/Kconfig: select HAVE_PERF_EVENTS
arch/parisc/Kconfig: select HAVE_PERF_EVENTS
arch/powerpc/Kconfig: select HAVE_PERF_EVENTS
arch/riscv/Kconfig: select HAVE_PERF_EVENTS
arch/s390/Kconfig: select HAVE_PERF_EVENTS
arch/sh/Kconfig: select HAVE_PERF_EVENTS
arch/sparc/Kconfig: select HAVE_PERF_EVENTS <--
arch/x86/Kconfig: select HAVE_PERF_EVENTS
arch/xtensa/Kconfig: select HAVE_PERF_EVENTS

Now, given ia64 is not an issue, I wanted to understand the semantics of
si_trapno. Per https://man7.org/linux/man-pages/man2/sigaction.2.html, I
see:

int si_trapno; /* Trap number that caused
hardware-generated signal
(unused on most architectures) */

... its intended semantics seem to suggest it would only be used by some
architecture-specific signal like you identified above. So if the
semantics is some code of a hardware trap/fault, then we're fine and do
not need to set it.

Also bearing in mind we define the semantics any new signal, and given
most architectures do not have si_trapno, definitions of new generic
signals should probably not include odd architecture specific details
related to old architectures.

From all this, my understanding now is that we can move si_trapno into
the union, correct? What else did you have in mind?

Thanks,
-- Marco

Eric W. Biederman

unread,
Apr 30, 2021, 4:15:36 PMApr 30
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
arm64 only abuses si_errno in compat code for bug compatibility with
arm32.

> Given it'd be wasted space otherwise, and we define the semantics of
> whatever is stored in siginfo on the new signal, it'd be good to keep.

Except you don't completely. You are not defining a new signal. You
are extending the definition of SIGTRAP. Anything generic that
responds to all SIGTRAPs can reasonably be looking at si_errno.

Further you are already adding a field with si_perf you can just as
easily add a second field with well defined semantics for that data.
Yes. Let's move si_trapno into the union.

That implies a few things like siginfo_layout needs to change.

The helpers in kernel/signal.c can change to not imply that
if you define __ARCH_SI_TRAPNO you must always define and
pass in si_trapno. A force_sig_trapno could be defined instead
to handle the cases that alpha and sparc use si_trapno.

It would be nice if a force_sig_perf_trap could be factored
out of perf_trap and placed in kernel/signal.c.

My experience (especially this round) is that it becomes much easier to
audit the users of siginfo if there is a dedicated function in
kernel/signal.c that is simply passed the parameters that need
to be placed in siginfo.

So I would very much like to see if I can make force_sig_info static.

Eric

Arnd Bergmann

unread,
Apr 30, 2021, 4:43:50 PMApr 30
to Eric W. Biederman, Marco Elver, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
On Fri, Apr 30, 2021 at 7:08 PM Eric W. Biederman <ebie...@xmission.com> wrote:
>
> The code is only safe if the analysis that says we can move si_trapno
> and perhaps the ia64 fields into the union is correct. It looks like
> ia64 much more actively uses it's signal extension fields including for
> SIGTRAP, so I am not at all certain the generic definition of
> perf_sigtrap is safe on ia64.
>
> > I suppose in theory sparc64 or alpha might start using the other
> > fields in the future, and an application might be compiled against
> > mismatched headers, but that is unlikely and is already broken
> > with the current headers.
>
> If we localize the use of si_trapno to just a few special cases on alpha
> and sparc I think we don't even need to worry about breaking userspace
> on any architecture. It will complicate siginfo_layout, but it is a
> complication that reflects reality.

Ok.

> I don't have a clue how any of this affects ia64. Does perf work on
> ia64? Does perf work on sparc, and alpha?
>
> If perf works on ia64 we need to take a hard look at what is going on
> there as well.

ia64 never had perf support. It had oprofile until very recently, and it
had a custom thing before that. My feeling is that it's increasingly
unlikely to ever gain perf support in the future, given that oprofile
(in user space) has required kernel perf support (in kernel) for a
long time and nobody cared about that being broken either.

sparc64 has perf support for Sun UltraSPARC 3/3+/3i/4+/T1/T2/T3
and Oracle SPARC T4/T5/M7 but lacks support for most CPUs from
Oracle, Fujitsu and the rest, in particular anything from the last
ten years.
Alpha has perf support for EV67, EV68, EV7, EV79, and EV69, i.e.
anything from 1996 to the end in 2004.

Arnd

Eric W. Biederman

unread,
Apr 30, 2021, 6:49:54 PMApr 30
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev

Eric W. Biederman (3):
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

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/sparc/kernel/process_64.c | 2 +-
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 +-
fs/signalfd.c | 7 +---
include/linux/compat.h | 4 +-
include/linux/sched/signal.h | 12 ++----
include/linux/signal.h | 1 +
include/uapi/asm-generic/siginfo.h | 6 +--
kernel/signal.c | 77 ++++++++++++++++++++++----------------
18 files changed, 107 insertions(+), 112 deletions(-)

Eric W. Biederman

unread,
Apr 30, 2021, 6:50:38 PMApr 30
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev

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.

Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
---
include/linux/compat.h | 4 +---
include/uapi/asm-generic/siginfo.h | 6 +-----
2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/include/linux/compat.h b/include/linux/compat.h
index f0d2dd35d408..24462ed63af4 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -214,12 +214,10 @@ 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 {
+ 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 03d6f6d2c1fe..2abdf1d19aad 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -63,9 +63,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 */
@@ -75,6 +72,7 @@ union __sifields {
#define __ADDR_BND_PKEY_PAD (__alignof__(void *) < sizeof(short) ? \
sizeof(short) : __alignof__(void *))
union {
+ int _trapno; /* TRAP # which caused the signal */
/*
* used when si_code=BUS_MCEERR_AR or
* used when si_code=BUS_MCEERR_AO
@@ -150,9 +148,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
--
2.30.1

Eric W. Biederman

unread,
Apr 30, 2021, 6:54:22 PMApr 30
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev

Now that si_trapno is part of the union in _si_fault and available on
all architectures, add SIL_FAULT_TRAPNO and update siginfo_layout to
return SIL_FAULT_TRAPNO when si_trapno is actually used.

Update the code that uses siginfo_layout to deal with SIL_FAULT_TRAPNO
and have the same code ignore si_trapno in in all other cases.

Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
---
fs/signalfd.c | 7 ++-----
include/linux/signal.h | 1 +
kernel/signal.c | 36 ++++++++++++++----------------------
3 files changed, 17 insertions(+), 27 deletions(-)

diff --git a/fs/signalfd.c b/fs/signalfd.c
index 040a1142915f..126c681a30e7 100644
--- a/fs/signalfd.c
+++ b/fs/signalfd.c
@@ -123,15 +123,12 @@ static int signalfd_copyinfo(struct signalfd_siginfo __user *uinfo,
*/
case SIL_FAULT:
new.ssi_addr = (long) kinfo->si_addr;
-#ifdef __ARCH_SI_TRAPNO
+ case SIL_FAULT_TRAPNO:
+ new.ssi_addr = (long) kinfo->si_addr;
new.ssi_trapno = kinfo->si_trapno;
-#endif
break;
case SIL_FAULT_MCEERR:
new.ssi_addr = (long) kinfo->si_addr;
-#ifdef __ARCH_SI_TRAPNO
- new.ssi_trapno = kinfo->si_trapno;
-#endif
new.ssi_addr_lsb = (short) kinfo->si_addr_lsb;
break;
case SIL_PERF_EVENT:
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 1e98548d7cf6..5160fd45e5ca 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -40,6 +40,7 @@ enum siginfo_layout {
SIL_TIMER,
SIL_POLL,
SIL_FAULT,
+ SIL_FAULT_TRAPNO,
SIL_FAULT_MCEERR,
SIL_FAULT_BNDERR,
SIL_FAULT_PKUERR,
diff --git a/kernel/signal.c b/kernel/signal.c
index c3017aa8024a..7b2d61cb7411 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1194,6 +1194,7 @@ static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
case SIL_TIMER:
case SIL_POLL:
case SIL_FAULT:
+ case SIL_FAULT_TRAPNO:
case SIL_FAULT_MCEERR:
case SIL_FAULT_BNDERR:
case SIL_FAULT_PKUERR:
@@ -2527,6 +2528,7 @@ static void hide_si_addr_tag_bits(struct ksignal *ksig)
{
switch (siginfo_layout(ksig->sig, ksig->info.si_code)) {
case SIL_FAULT:
+ case SIL_FAULT_TRAPNO:
case SIL_FAULT_MCEERR:
case SIL_FAULT_BNDERR:
case SIL_FAULT_PKUERR:
@@ -3206,6 +3208,12 @@ enum siginfo_layout siginfo_layout(unsigned sig, int si_code)
if ((sig == SIGBUS) &&
(si_code >= BUS_MCEERR_AR) && (si_code <= BUS_MCEERR_AO))
layout = SIL_FAULT_MCEERR;
+ else if (IS_ENABLED(ALPHA) &&
+ ((sig == SIGFPE) ||
+ ((sig == SIGTRAP) && (si_code == TRAP_UNK))))
+ layout = SIL_FAULT_TRAPNO;
+ else if (IS_ENABLED(SPARC) && (sig == SIGILL) && (si_code == ILL_ILLTRP))
+ layout = SIL_FAULT_TRAPNO;
else if ((sig == SIGSEGV) && (si_code == SEGV_BNDERR))
layout = SIL_FAULT_BNDERR;
#ifdef SEGV_PKUERR
@@ -3317,30 +3325,22 @@ void copy_siginfo_to_external32(struct compat_siginfo *to,
break;
case SIL_FAULT:
to->si_addr = ptr_to_compat(from->si_addr);
-#ifdef __ARCH_SI_TRAPNO
+ break;
+ case SIL_FAULT_TRAPNO:
+ to->si_addr = ptr_to_compat(from->si_addr);
to->si_trapno = from->si_trapno;
-#endif
break;
case SIL_FAULT_MCEERR:
to->si_addr = ptr_to_compat(from->si_addr);
-#ifdef __ARCH_SI_TRAPNO
- to->si_trapno = from->si_trapno;
-#endif
to->si_addr_lsb = from->si_addr_lsb;
break;
case SIL_FAULT_BNDERR:
to->si_addr = ptr_to_compat(from->si_addr);
-#ifdef __ARCH_SI_TRAPNO
- to->si_trapno = from->si_trapno;
-#endif
to->si_lower = ptr_to_compat(from->si_lower);
to->si_upper = ptr_to_compat(from->si_upper);
break;
case SIL_FAULT_PKUERR:
to->si_addr = ptr_to_compat(from->si_addr);
-#ifdef __ARCH_SI_TRAPNO
- to->si_trapno = from->si_trapno;
-#endif
to->si_pkey = from->si_pkey;
break;
case SIL_PERF_EVENT:
@@ -3401,30 +3401,22 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to,
break;
case SIL_FAULT:
to->si_addr = compat_ptr(from->si_addr);
-#ifdef __ARCH_SI_TRAPNO
+ break;
+ case SIL_FAULT_TRAPNO:
+ to->si_addr = compat_ptr(from->si_addr);
to->si_trapno = from->si_trapno;
-#endif
break;
case SIL_FAULT_MCEERR:
to->si_addr = compat_ptr(from->si_addr);
-#ifdef __ARCH_SI_TRAPNO
- to->si_trapno = from->si_trapno;
-#endif
to->si_addr_lsb = from->si_addr_lsb;
break;
case SIL_FAULT_BNDERR:
to->si_addr = compat_ptr(from->si_addr);
-#ifdef __ARCH_SI_TRAPNO
- to->si_trapno = from->si_trapno;
-#endif
to->si_lower = compat_ptr(from->si_lower);
to->si_upper = compat_ptr(from->si_upper);
break;
case SIL_FAULT_PKUERR:
to->si_addr = compat_ptr(from->si_addr);
-#ifdef __ARCH_SI_TRAPNO
- to->si_trapno = from->si_trapno;
-#endif
to->si_pkey = from->si_pkey;
break;
case SIL_PERF_EVENT:
--
2.30.1

Eric W. Biederman

unread,
Apr 30, 2021, 6:55:36 PMApr 30
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev

Now that si_trapno is no longer expected to be present for every fault
reported using siginfo on alpha and sparc remove the trapno parameter
from force_sig_fault, force_sig_fault_to_task and send_sig_fault.

Add two new helpers force_sig_fault_trapno and send_sig_fault_trapno
for those signals where trapno is expected to be set.

Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
---
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/sparc/kernel/process_64.c | 2 +-
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 +-
include/linux/sched/signal.h | 12 +++------
kernel/signal.c | 41 +++++++++++++++++++++--------
14 files changed, 88 insertions(+), 77 deletions(-)

diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
index d5367a1c6300..80c5d7fbe66a 100644
--- a/arch/alpha/kernel/osf_sys.c
+++ b/arch/alpha/kernel/osf_sys.c
@@ -878,7 +878,7 @@ SYSCALL_DEFINE5(osf_setsysinfo, unsigned long, op, void __user *, buffer,

send_sig_fault(SIGFPE, si_code,
(void __user *)NULL, /* FIXME */
- 0, current);
+ current);
}
return 0;
}
diff --git a/arch/alpha/kernel/signal.c b/arch/alpha/kernel/signal.c
index 948b89789da8..bc077babafab 100644
--- a/arch/alpha/kernel/signal.c
+++ b/arch/alpha/kernel/signal.c
@@ -219,7 +219,7 @@ do_sigreturn(struct sigcontext __user *sc)

/* Send SIGTRAP if we're single-stepping: */
if (ptrace_cancel_bpt (current)) {
- send_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *) regs->pc, 0,
+ send_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *) regs->pc,
current);
}
return;
@@ -247,7 +247,7 @@ do_rt_sigreturn(struct rt_sigframe __user *frame)

/* Send SIGTRAP if we're single-stepping: */
if (ptrace_cancel_bpt (current)) {
- send_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *) regs->pc, 0,
+ send_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *) regs->pc,
current);
}
return;
diff --git a/arch/alpha/kernel/traps.c b/arch/alpha/kernel/traps.c
index 921d4b6e4d95..0dddf9ecc1f4 100644
--- a/arch/alpha/kernel/traps.c
+++ b/arch/alpha/kernel/traps.c
@@ -227,7 +227,7 @@ do_entArith(unsigned long summary, unsigned long write_mask,
}
die_if_kernel("Arithmetic fault", regs, 0, NULL);

- send_sig_fault(SIGFPE, si_code, (void __user *) regs->pc, 0, current);
+ send_sig_fault_trapno(SIGFPE, si_code, (void __user *) regs->pc, 0, current);
}

asmlinkage void
@@ -268,12 +268,12 @@ do_entIF(unsigned long type, struct pt_regs *regs)
regs->pc -= 4; /* make pc point to former bpt */
}

- send_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->pc, 0,
+ send_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->pc,
current);
return;

case 1: /* bugcheck */
- send_sig_fault(SIGTRAP, TRAP_UNK, (void __user *) regs->pc, 0,
+ send_sig_fault(SIGTRAP, TRAP_UNK, (void __user *) regs->pc,
current);
return;

@@ -335,8 +335,8 @@ do_entIF(unsigned long type, struct pt_regs *regs)
break;
}

- send_sig_fault(signo, code, (void __user *) regs->pc, regs->r16,
- current);
+ send_sig_fault_trapno(signo, code, (void __user *) regs->pc,
+ regs->r16, current);
return;

case 4: /* opDEC */
@@ -360,9 +360,9 @@ do_entIF(unsigned long type, struct pt_regs *regs)
if (si_code == 0)
return;
if (si_code > 0) {
- send_sig_fault(SIGFPE, si_code,
- (void __user *) regs->pc, 0,
- current);
+ send_sig_fault_trapno(SIGFPE, si_code,
+ (void __user *) regs->pc,
+ 0, current);
return;
}
}
@@ -387,7 +387,7 @@ do_entIF(unsigned long type, struct pt_regs *regs)
;
}

- send_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)regs->pc, 0, current);
+ send_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)regs->pc, current);
}

/* There is an ifdef in the PALcode in MILO that enables a
@@ -402,7 +402,7 @@ do_entDbg(struct pt_regs *regs)
{
die_if_kernel("Instruction fault", regs, 0, NULL);

- force_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)regs->pc, 0);
+ force_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)regs->pc);
}


@@ -964,12 +964,12 @@ do_entUnaUser(void __user * va, unsigned long opcode,
si_code = SEGV_MAPERR;
mmap_read_unlock(mm);
}
- send_sig_fault(SIGSEGV, si_code, va, 0, current);
+ send_sig_fault(SIGSEGV, si_code, va, current);
return;

give_sigbus:
regs->pc -= 4;
- send_sig_fault(SIGBUS, BUS_ADRALN, va, 0, current);
+ send_sig_fault(SIGBUS, BUS_ADRALN, va, current);
return;
}

diff --git a/arch/alpha/mm/fault.c b/arch/alpha/mm/fault.c
index 09172f017efc..eee5102c3d88 100644
--- a/arch/alpha/mm/fault.c
+++ b/arch/alpha/mm/fault.c
@@ -219,13 +219,13 @@ do_page_fault(unsigned long address, unsigned long mmcsr,
mmap_read_unlock(mm);
/* Send a sigbus, regardless of whether we were in kernel
or user mode. */
- force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *) address, 0);
+ force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *) address);
if (!user_mode(regs))
goto no_context;
return;

do_sigsegv:
- force_sig_fault(SIGSEGV, si_code, (void __user *) address, 0);
+ force_sig_fault(SIGSEGV, si_code, (void __user *) address);
return;

#ifdef CONFIG_ALPHA_LARGE_VMALLOC
diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c
index 7afd0a859a78..29e67854d5a4 100644
--- a/arch/sparc/kernel/process_64.c
+++ b/arch/sparc/kernel/process_64.c
@@ -518,7 +518,7 @@ void synchronize_user_stack(void)

static void stack_unaligned(unsigned long sp)
{
- force_sig_fault(SIGBUS, BUS_ADRALN, (void __user *) sp, 0);
+ force_sig_fault(SIGBUS, BUS_ADRALN, (void __user *) sp);
}

static const char uwfault32[] = KERN_INFO \
diff --git a/arch/sparc/kernel/sys_sparc_32.c b/arch/sparc/kernel/sys_sparc_32.c
index be77538bc038..082a551897ed 100644
--- a/arch/sparc/kernel/sys_sparc_32.c
+++ b/arch/sparc/kernel/sys_sparc_32.c
@@ -151,7 +151,7 @@ sparc_breakpoint (struct pt_regs *regs)
#ifdef DEBUG_SPARC_BREAKPOINT
printk ("TRAP: Entering kernel PC=%x, nPC=%x\n", regs->pc, regs->npc);
#endif
- force_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->pc, 0);
+ force_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->pc);

#ifdef DEBUG_SPARC_BREAKPOINT
printk ("TRAP: Returning to space: PC=%x nPC=%x\n", regs->pc, regs->npc);
diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
index 6b92fadb6ec7..1e9a9e016237 100644
--- a/arch/sparc/kernel/sys_sparc_64.c
+++ b/arch/sparc/kernel/sys_sparc_64.c
@@ -514,7 +514,7 @@ asmlinkage void sparc_breakpoint(struct pt_regs *regs)
#ifdef DEBUG_SPARC_BREAKPOINT
printk ("TRAP: Entering kernel PC=%lx, nPC=%lx\n", regs->tpc, regs->tnpc);
#endif
- force_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->tpc, 0);
+ force_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->tpc);
#ifdef DEBUG_SPARC_BREAKPOINT
printk ("TRAP: Returning to space: PC=%lx nPC=%lx\n", regs->tpc, regs->tnpc);
#endif
diff --git a/arch/sparc/kernel/traps_32.c b/arch/sparc/kernel/traps_32.c
index 247a0d9683b2..5630e5a395e0 100644
--- a/arch/sparc/kernel/traps_32.c
+++ b/arch/sparc/kernel/traps_32.c
@@ -102,8 +102,8 @@ void do_hw_interrupt(struct pt_regs *regs, unsigned long type)
if(regs->psr & PSR_PS)
die_if_kernel("Kernel bad trap", regs);

- force_sig_fault(SIGILL, ILL_ILLTRP,
- (void __user *)regs->pc, type - 0x80);
+ force_sig_fault_trapno(SIGILL, ILL_ILLTRP,
+ (void __user *)regs->pc, type - 0x80);
}

void do_illegal_instruction(struct pt_regs *regs, unsigned long pc, unsigned long npc,
@@ -116,7 +116,7 @@ void do_illegal_instruction(struct pt_regs *regs, unsigned long pc, unsigned lon
regs->pc, *(unsigned long *)regs->pc);
#endif

- send_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)pc, 0, current);
+ send_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)pc, current);
}

void do_priv_instruction(struct pt_regs *regs, unsigned long pc, unsigned long npc,
@@ -124,7 +124,7 @@ void do_priv_instruction(struct pt_regs *regs, unsigned long pc, unsigned long n
{
if(psr & PSR_PS)
die_if_kernel("Penguin instruction from Penguin mode??!?!", regs);
- send_sig_fault(SIGILL, ILL_PRVOPC, (void __user *)pc, 0, current);
+ send_sig_fault(SIGILL, ILL_PRVOPC, (void __user *)pc, current);
}

/* XXX User may want to be allowed to do this. XXX */
@@ -145,7 +145,7 @@ void do_memaccess_unaligned(struct pt_regs *regs, unsigned long pc, unsigned lon
#endif
send_sig_fault(SIGBUS, BUS_ADRALN,
/* FIXME: Should dig out mna address */ (void *)0,
- 0, current);
+ current);
}

static unsigned long init_fsr = 0x0UL;
@@ -291,7 +291,7 @@ void do_fpe_trap(struct pt_regs *regs, unsigned long pc, unsigned long npc,
else if (fsr & 0x01)
code = FPE_FLTRES;
}
- send_sig_fault(SIGFPE, code, (void __user *)pc, 0, fpt);
+ send_sig_fault(SIGFPE, code, (void __user *)pc, fpt);
#ifndef CONFIG_SMP
last_task_used_math = NULL;
#endif
@@ -305,7 +305,7 @@ void handle_tag_overflow(struct pt_regs *regs, unsigned long pc, unsigned long n
{
if(psr & PSR_PS)
die_if_kernel("Penguin overflow trap from kernel mode", regs);
- send_sig_fault(SIGEMT, EMT_TAGOVF, (void __user *)pc, 0, current);
+ send_sig_fault(SIGEMT, EMT_TAGOVF, (void __user *)pc, current);
}

void handle_watchpoint(struct pt_regs *regs, unsigned long pc, unsigned long npc,
@@ -327,13 +327,13 @@ void handle_reg_access(struct pt_regs *regs, unsigned long pc, unsigned long npc
printk("Register Access Exception at PC %08lx NPC %08lx PSR %08lx\n",
pc, npc, psr);
#endif
- force_sig_fault(SIGBUS, BUS_OBJERR, (void __user *)pc, 0);
+ force_sig_fault(SIGBUS, BUS_OBJERR, (void __user *)pc);
}

void handle_cp_disabled(struct pt_regs *regs, unsigned long pc, unsigned long npc,
unsigned long psr)
{
- send_sig_fault(SIGILL, ILL_COPROC, (void __user *)pc, 0, current);
+ send_sig_fault(SIGILL, ILL_COPROC, (void __user *)pc, current);
}

void handle_cp_exception(struct pt_regs *regs, unsigned long pc, unsigned long npc,
@@ -343,13 +343,13 @@ void handle_cp_exception(struct pt_regs *regs, unsigned long pc, unsigned long n
printk("Co-Processor Exception at PC %08lx NPC %08lx PSR %08lx\n",
pc, npc, psr);
#endif
- send_sig_fault(SIGILL, ILL_COPROC, (void __user *)pc, 0, current);
+ send_sig_fault(SIGILL, ILL_COPROC, (void __user *)pc, current);
}

void handle_hw_divzero(struct pt_regs *regs, unsigned long pc, unsigned long npc,
unsigned long psr)
{
- send_sig_fault(SIGFPE, FPE_INTDIV, (void __user *)pc, 0, current);
+ send_sig_fault(SIGFPE, FPE_INTDIV, (void __user *)pc, current);
}

#ifdef CONFIG_DEBUG_BUGVERBOSE
diff --git a/arch/sparc/kernel/traps_64.c b/arch/sparc/kernel/traps_64.c
index a850dccd78ea..6863025ed56d 100644
--- a/arch/sparc/kernel/traps_64.c
+++ b/arch/sparc/kernel/traps_64.c
@@ -107,8 +107,8 @@ void bad_trap(struct pt_regs *regs, long lvl)
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGILL, ILL_ILLTRP,
- (void __user *)regs->tpc, lvl);
+ force_sig_fault_trapno(SIGILL, ILL_ILLTRP,
+ (void __user *)regs->tpc, lvl);
}

void bad_trap_tl1(struct pt_regs *regs, long lvl)
@@ -201,8 +201,7 @@ void spitfire_insn_access_exception(struct pt_regs *regs, unsigned long sfsr, un
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGSEGV, SEGV_MAPERR,
- (void __user *)regs->tpc, 0);
+ force_sig_fault(SIGSEGV, SEGV_MAPERR, (void __user *)regs->tpc);
out:
exception_exit(prev_state);
}
@@ -237,7 +236,7 @@ void sun4v_insn_access_exception(struct pt_regs *regs, unsigned long addr, unsig
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGSEGV, SEGV_MAPERR, (void __user *) addr, 0);
+ force_sig_fault(SIGSEGV, SEGV_MAPERR, (void __user *) addr);
}

void sun4v_insn_access_exception_tl1(struct pt_regs *regs, unsigned long addr, unsigned long type_ctx)
@@ -321,7 +320,7 @@ void spitfire_data_access_exception(struct pt_regs *regs, unsigned long sfsr, un
if (is_no_fault_exception(regs))
return;

- force_sig_fault(SIGSEGV, SEGV_MAPERR, (void __user *)sfar, 0);
+ force_sig_fault(SIGSEGV, SEGV_MAPERR, (void __user *)sfar);
out:
exception_exit(prev_state);
}
@@ -385,13 +384,13 @@ void sun4v_data_access_exception(struct pt_regs *regs, unsigned long addr, unsig
*/
switch (type) {
case HV_FAULT_TYPE_INV_ASI:
- force_sig_fault(SIGILL, ILL_ILLADR, (void __user *)addr, 0);
+ force_sig_fault(SIGILL, ILL_ILLADR, (void __user *)addr);
break;
case HV_FAULT_TYPE_MCD_DIS:
- force_sig_fault(SIGSEGV, SEGV_ACCADI, (void __user *)addr, 0);
+ force_sig_fault(SIGSEGV, SEGV_ACCADI, (void __user *)addr);
break;
default:
- force_sig_fault(SIGSEGV, SEGV_MAPERR, (void __user *)addr, 0);
+ force_sig_fault(SIGSEGV, SEGV_MAPERR, (void __user *)addr);
break;
}
}
@@ -568,7 +567,7 @@ static void spitfire_ue_log(unsigned long afsr, unsigned long afar, unsigned lon
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGBUS, BUS_OBJERR, (void *)0, 0);
+ force_sig_fault(SIGBUS, BUS_OBJERR, (void *)0);
}

void spitfire_access_error(struct pt_regs *regs, unsigned long status_encoded, unsigned long afar)
@@ -2069,8 +2068,7 @@ void do_mcd_err(struct pt_regs *regs, struct sun4v_error_entry ent)
/* Send SIGSEGV to the userspace process with the right signal
* code
*/
- force_sig_fault(SIGSEGV, SEGV_ADIDERR, (void __user *)ent.err_raddr,
- 0);
+ force_sig_fault(SIGSEGV, SEGV_ADIDERR, (void __user *)ent.err_raddr);
}

/* We run with %pil set to PIL_NORMAL_MAX and PSTATE_IE enabled in %pstate.
@@ -2184,7 +2182,7 @@ bool sun4v_nonresum_error_user_handled(struct pt_regs *regs,
}
if (attrs & SUN4V_ERR_ATTRS_PIO) {
force_sig_fault(SIGBUS, BUS_ADRERR,
- (void __user *)sun4v_get_vaddr(regs), 0);
+ (void __user *)sun4v_get_vaddr(regs));
return true;
}

@@ -2340,8 +2338,7 @@ static void do_fpe_common(struct pt_regs *regs)
else if (fsr & 0x01)
code = FPE_FLTRES;
}
- force_sig_fault(SIGFPE, code,
- (void __user *)regs->tpc, 0);
+ force_sig_fault(SIGFPE, code, (void __user *)regs->tpc);
}
}

@@ -2395,8 +2392,7 @@ void do_tof(struct pt_regs *regs)
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGEMT, EMT_TAGOVF,
- (void __user *)regs->tpc, 0);
+ force_sig_fault(SIGEMT, EMT_TAGOVF, (void __user *)regs->tpc);
out:
exception_exit(prev_state);
}
@@ -2415,8 +2411,7 @@ void do_div0(struct pt_regs *regs)
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGFPE, FPE_INTDIV,
- (void __user *)regs->tpc, 0);
+ force_sig_fault(SIGFPE, FPE_INTDIV, (void __user *)regs->tpc);
out:
exception_exit(prev_state);
}
@@ -2612,7 +2607,7 @@ void do_illegal_instruction(struct pt_regs *regs)
}
}
}
- force_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)pc, 0);
+ force_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)pc);
out:
exception_exit(prev_state);
}
@@ -2632,7 +2627,7 @@ void mem_address_unaligned(struct pt_regs *regs, unsigned long sfar, unsigned lo
if (is_no_fault_exception(regs))
return;

- force_sig_fault(SIGBUS, BUS_ADRALN, (void __user *)sfar, 0);
+ force_sig_fault(SIGBUS, BUS_ADRALN, (void __user *)sfar);
out:
exception_exit(prev_state);
}
@@ -2650,7 +2645,7 @@ void sun4v_do_mna(struct pt_regs *regs, unsigned long addr, unsigned long type_c
if (is_no_fault_exception(regs))
return;

- force_sig_fault(SIGBUS, BUS_ADRALN, (void __user *) addr, 0);
+ force_sig_fault(SIGBUS, BUS_ADRALN, (void __user *) addr);
}

/* sun4v_mem_corrupt_detect_precise() - Handle precise exception on an ADI
@@ -2697,7 +2692,7 @@ void sun4v_mem_corrupt_detect_precise(struct pt_regs *regs, unsigned long addr,
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGSEGV, SEGV_ADIPERR, (void __user *)addr, 0);
+ force_sig_fault(SIGSEGV, SEGV_ADIPERR, (void __user *)addr);
}

void do_privop(struct pt_regs *regs)
@@ -2712,8 +2707,7 @@ void do_privop(struct pt_regs *regs)
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGILL, ILL_PRVOPC,
- (void __user *)regs->tpc, 0);
+ force_sig_fault(SIGILL, ILL_PRVOPC, (void __user *)regs->tpc);
out:
exception_exit(prev_state);
}
diff --git a/arch/sparc/kernel/unaligned_32.c b/arch/sparc/kernel/unaligned_32.c
index ef5c5207c9ff..455f0258c745 100644
--- a/arch/sparc/kernel/unaligned_32.c
+++ b/arch/sparc/kernel/unaligned_32.c
@@ -278,5 +278,5 @@ asmlinkage void user_unaligned_trap(struct pt_regs *regs, unsigned int insn)
{
send_sig_fault(SIGBUS, BUS_ADRALN,
(void __user *)safe_compute_effective_address(regs, insn),
- 0, current);
+ current);
}
diff --git a/arch/sparc/mm/fault_32.c b/arch/sparc/mm/fault_32.c
index de2031c2b2d7..fa858626b85b 100644
--- a/arch/sparc/mm/fault_32.c
+++ b/arch/sparc/mm/fault_32.c
@@ -83,7 +83,7 @@ static void __do_fault_siginfo(int code, int sig, struct pt_regs *regs,
show_signal_msg(regs, sig, code,
addr, current);

- force_sig_fault(sig, code, (void __user *) addr, 0);
+ force_sig_fault(sig, code, (void __user *) addr);
}

static unsigned long compute_si_addr(struct pt_regs *regs, int text_fault)
diff --git a/arch/sparc/mm/fault_64.c b/arch/sparc/mm/fault_64.c
index 0a6bcc85fba7..9a9652a15fed 100644
--- a/arch/sparc/mm/fault_64.c
+++ b/arch/sparc/mm/fault_64.c
@@ -176,7 +176,7 @@ static void do_fault_siginfo(int code, int sig, struct pt_regs *regs,
if (unlikely(show_unhandled_signals))
show_signal_msg(regs, sig, code, addr, current);

- force_sig_fault(sig, code, (void __user *) addr, 0);
+ force_sig_fault(sig, code, (void __user *) addr);
}

static unsigned int get_fault_insn(struct pt_regs *regs, unsigned int insn)
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 3f6a0fcaa10c..7daa425f3055 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -298,11 +298,6 @@ static inline void kernel_signal_stop(void)

schedule();
}
-#ifdef __ARCH_SI_TRAPNO
-# define ___ARCH_SI_TRAPNO(_a1) , _a1
-#else
-# define ___ARCH_SI_TRAPNO(_a1)
-#endif
#ifdef __ia64__
# define ___ARCH_SI_IA64(_a1, _a2, _a3) , _a1, _a2, _a3
#else
@@ -310,14 +305,11 @@ static inline void kernel_signal_stop(void)
#endif

int force_sig_fault_to_task(int sig, int code, void __user *addr
- ___ARCH_SI_TRAPNO(int trapno)
___ARCH_SI_IA64(int imm, unsigned int flags, unsigned long isr)
, struct task_struct *t);
int force_sig_fault(int sig, int code, void __user *addr
- ___ARCH_SI_TRAPNO(int trapno)
___ARCH_SI_IA64(int imm, unsigned int flags, unsigned long isr));
int send_sig_fault(int sig, int code, void __user *addr
- ___ARCH_SI_TRAPNO(int trapno)
___ARCH_SI_IA64(int imm, unsigned int flags, unsigned long isr)
, struct task_struct *t);

@@ -327,6 +319,10 @@ int send_sig_mceerr(int code, void __user *, short, struct task_struct *);
int force_sig_bnderr(void __user *addr, void __user *lower, void __user *upper);
int force_sig_pkuerr(void __user *addr, u32 pkey);

+int force_sig_fault_trapno(int sig, int code, void __user *addr, int trapno);
+int send_sig_fault_trapno(int sig, int code, void __user *addr, int trapno,
+ struct task_struct *task);
+
int force_sig_ptrace_errno_trap(int errno, void __user *addr);

extern int send_sig_info(int, struct kernel_siginfo *, struct task_struct *);
diff --git a/kernel/signal.c b/kernel/signal.c
index 7b2d61cb7411..0517ff950d38 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1651,7 +1651,6 @@ void force_sigsegv(int sig)
}

int force_sig_fault_to_task(int sig, int code, void __user *addr
- ___ARCH_SI_TRAPNO(int trapno)
___ARCH_SI_IA64(int imm, unsigned int flags, unsigned long isr)
, struct task_struct *t)
{
@@ -1662,9 +1661,6 @@ int force_sig_fault_to_task(int sig, int code, void __user *addr
info.si_errno = 0;
info.si_code = code;
info.si_addr = addr;
-#ifdef __ARCH_SI_TRAPNO
- info.si_trapno = trapno;
-#endif
#ifdef __ia64__
info.si_imm = imm;
info.si_flags = flags;
@@ -1674,16 +1670,13 @@ int force_sig_fault_to_task(int sig, int code, void __user *addr
}

int force_sig_fault(int sig, int code, void __user *addr
- ___ARCH_SI_TRAPNO(int trapno)
___ARCH_SI_IA64(int imm, unsigned int flags, unsigned long isr))
{
return force_sig_fault_to_task(sig, code, addr
- ___ARCH_SI_TRAPNO(trapno)
___ARCH_SI_IA64(imm, flags, isr), current);
}

int send_sig_fault(int sig, int code, void __user *addr
- ___ARCH_SI_TRAPNO(int trapno)
___ARCH_SI_IA64(int imm, unsigned int flags, unsigned long isr)
, struct task_struct *t)
{
@@ -1694,9 +1687,6 @@ int send_sig_fault(int sig, int code, void __user *addr
info.si_errno = 0;
info.si_code = code;
info.si_addr = addr;
-#ifdef __ARCH_SI_TRAPNO
- info.si_trapno = trapno;
-#endif
#ifdef __ia64__
info.si_imm = imm;
info.si_flags = flags;
@@ -1763,6 +1753,37 @@ int force_sig_pkuerr(void __user *addr, u32 pkey)
}
#endif

+#if IS_ENABLED(SPARC)
+int force_sig_fault_trapno(int sig, int code, void __user *addr, int trapno)
+{
+ struct kernel_siginfo info;
+
+ clear_siginfo(&info);
+ info.si_signo = sig;
+ info.si_errno = 0;
+ info.si_code = code;
+ info.si_addr = addr;
+ info.si_trapno = trapno;
+ return force_sig_info(&info);
+}
+#endif
+
+#if IS_ENABLED(ALPHA)
+int send_sig_fault_trapno(int sig, int code, void __user *addr, int trapno,
+ struct task_struct *t)
+{
+ struct kernel_siginfo info;
+
+ clear_siginfo(&info);
+ info.si_signo = sig;
+ info.si_errno = 0;
+ info.si_code = code;
+ info.si_addr = addr;
+ info.si_trapno = trapno;
+ return send_sig_info(info.si_signo, &info, t);
+}
+#endif
+
/* For the crazy architectures that include trap information in
* the errno field, instead of an actual errno value.
*/
--
2.30.1

Eric W. Biederman

unread,
Apr 30, 2021, 6:57:00 PMApr 30
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev

Now that this define is no longer used remove it from the kernel.

Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
---
arch/alpha/include/uapi/asm/siginfo.h | 2 --
arch/mips/include/uapi/asm/siginfo.h | 2 --
arch/sparc/include/uapi/asm/siginfo.h | 3 ---
3 files changed, 7 deletions(-)

diff --git a/arch/alpha/include/uapi/asm/siginfo.h b/arch/alpha/include/uapi/asm/siginfo.h
index 6e1a2af2f962..e08eae88182b 100644
--- a/arch/alpha/include/uapi/asm/siginfo.h
+++ b/arch/alpha/include/uapi/asm/siginfo.h
@@ -2,8 +2,6 @@
#ifndef _ALPHA_SIGINFO_H
#define _ALPHA_SIGINFO_H

-#define __ARCH_SI_TRAPNO
-
#include <asm-generic/siginfo.h>

#endif
diff --git a/arch/mips/include/uapi/asm/siginfo.h b/arch/mips/include/uapi/asm/siginfo.h
index c34c7eef0a1c..8cb8bd061a68 100644
--- a/arch/mips/include/uapi/asm/siginfo.h
+++ b/arch/mips/include/uapi/asm/siginfo.h
@@ -10,9 +10,7 @@
#ifndef _UAPI_ASM_SIGINFO_H
#define _UAPI_ASM_SIGINFO_H

-
#define __ARCH_SIGEV_PREAMBLE_SIZE (sizeof(long) + 2*sizeof(int))
-#undef __ARCH_SI_TRAPNO /* exception code needs to fill this ... */

#define __ARCH_HAS_SWAPPED_SIGINFO

diff --git a/arch/sparc/include/uapi/asm/siginfo.h b/arch/sparc/include/uapi/asm/siginfo.h
index 68bdde4c2a2e..0e7c27522aed 100644
--- a/arch/sparc/include/uapi/asm/siginfo.h
+++ b/arch/sparc/include/uapi/asm/siginfo.h
@@ -8,9 +8,6 @@

#endif /* defined(__sparc__) && defined(__arch64__) */

-
-#define __ARCH_SI_TRAPNO
-
#include <asm-generic/siginfo.h>


--
2.30.1

Eric W. Biederman

unread,
Apr 30, 2021, 7:20:10 PMApr 30
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
ebie...@xmission.com (Eric W. Biederman) writes:

> Now that si_trapno is part of the union in _si_fault and available on
> all architectures, add SIL_FAULT_TRAPNO and update siginfo_layout to
> return SIL_FAULT_TRAPNO when si_trapno is actually used.
>
> Update the code that uses siginfo_layout to deal with SIL_FAULT_TRAPNO
> and have the same code ignore si_trapno in in all other cases.

This change is missing a break in signalfd.

Eric

> Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
> ---
> fs/signalfd.c | 7 ++-----
> include/linux/signal.h | 1 +
> kernel/signal.c | 36 ++++++++++++++----------------------
> 3 files changed, 17 insertions(+), 27 deletions(-)
>
> diff --git a/fs/signalfd.c b/fs/signalfd.c
> index 040a1142915f..126c681a30e7 100644
> --- a/fs/signalfd.c
> +++ b/fs/signalfd.c
> @@ -123,15 +123,12 @@ static int signalfd_copyinfo(struct signalfd_siginfo __user *uinfo,
> */
> case SIL_FAULT:
> new.ssi_addr = (long) kinfo->si_addr;
+ break;

Eric W. Biederman

unread,
Apr 30, 2021, 7:23:57 PMApr 30
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev

I am looking at perf_sigtrap and I am confused by the code.


/*
* We'd expect this to only occur if the irq_work is delayed and either
* ctx->task or current has changed in the meantime. This can be the
* case on architectures that do not implement arch_irq_work_raise().
*/
if (WARN_ON_ONCE(event->ctx->task != current))
return;

/*
* perf_pending_event() can race with the task exiting.
*/
if (current->flags & PF_EXITING)
return;


It performs tests that absolutely can never fail if we are talking about
a synchronous exception. The code force_sig family of functions only
make sense to use with and are only safe to use with synchronous
exceptions.

Are the tests in perf_sigtrap necessary or is perf_sigtrap not reporting
a synchronous event?

Eric

Eric W. Biederman

unread,
Apr 30, 2021, 7:42:59 PMApr 30
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev

It helps to know which part of the siginfo structure the siginfo_layout
value is talking about.
---
fs/signalfd.c | 2 +-
include/linux/signal.h | 2 +-
kernel/signal.c | 10 +++++-----
3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/signalfd.c b/fs/signalfd.c
index e87e59581653..83130244f653 100644
--- a/fs/signalfd.c
+++ b/fs/signalfd.c
@@ -132,7 +132,7 @@ static int signalfd_copyinfo(struct signalfd_siginfo __user *uinfo,
new.ssi_addr = (long) kinfo->si_addr;
new.ssi_addr_lsb = (short) kinfo->si_addr_lsb;
break;
- case SIL_PERF_EVENT:
+ case SIL_FAULT_PERF_EVENT:
new.ssi_addr = (long) kinfo->si_addr;
new.ssi_perf = kinfo->si_perf;
break;
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 5160fd45e5ca..ed896d790e46 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -44,7 +44,7 @@ enum siginfo_layout {
SIL_FAULT_MCEERR,
SIL_FAULT_BNDERR,
SIL_FAULT_PKUERR,
- SIL_PERF_EVENT,
+ SIL_FAULT_PERF_EVENT,
SIL_CHLD,
SIL_RT,
SIL_SYS,
diff --git a/kernel/signal.c b/kernel/signal.c
index 0517ff950d38..690921960d8b 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1198,7 +1198,7 @@ static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
case SIL_FAULT_MCEERR:
case SIL_FAULT_BNDERR:
case SIL_FAULT_PKUERR:
- case SIL_PERF_EVENT:
+ case SIL_FAULT_PERF_EVENT:
case SIL_SYS:
ret = false;
break;
@@ -2553,7 +2553,7 @@ static void hide_si_addr_tag_bits(struct ksignal *ksig)
case SIL_FAULT_MCEERR:
case SIL_FAULT_BNDERR:
case SIL_FAULT_PKUERR:
- case SIL_PERF_EVENT:
+ case SIL_FAULT_PERF_EVENT:
ksig->info.si_addr = arch_untagged_si_addr(
ksig->info.si_addr, ksig->sig, ksig->info.si_code);
break;
@@ -3242,7 +3242,7 @@ enum siginfo_layout siginfo_layout(unsigned sig, int si_code)
layout = SIL_FAULT_PKUERR;
#endif
else if ((sig == SIGTRAP) && (si_code == TRAP_PERF))
- layout = SIL_PERF_EVENT;
+ layout = SIL_FAULT_PERF_EVENT;
}
else if (si_code <= NSIGPOLL)
layout = SIL_POLL;
@@ -3364,7 +3364,7 @@ void copy_siginfo_to_external32(struct compat_siginfo *to,
to->si_addr = ptr_to_compat(from->si_addr);
to->si_pkey = from->si_pkey;
break;
- case SIL_PERF_EVENT:
+ case SIL_FAULT_PERF_EVENT:
to->si_addr = ptr_to_compat(from->si_addr);
to->si_perf = from->si_perf;
break;
@@ -3440,7 +3440,7 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to,
to->si_addr = compat_ptr(from->si_addr);
to->si_pkey = from->si_pkey;
break;
- case SIL_PERF_EVENT:
+ case SIL_FAULT_PERF_EVENT:
to->si_addr = compat_ptr(from->si_addr);
to->si_perf = from->si_perf;
break;
--
2.30.1

Eric W. Biederman

unread,
Apr 30, 2021, 7:43:29 PMApr 30
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev

Separate generating the signal from deciding it needs to be sent.

Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
---
include/linux/sched/signal.h | 1 +
kernel/events/core.c | 11 ++---------
kernel/signal.c | 13 +++++++++++++
3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 7daa425f3055..1e2f61a1a512 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -318,6 +318,7 @@ int send_sig_mceerr(int code, void __user *, short, struct task_struct *);

int force_sig_bnderr(void __user *addr, void __user *lower, void __user *upper);
int force_sig_pkuerr(void __user *addr, u32 pkey);
+int force_sig_perf(void __user *addr, u32 type, u64 sig_data);

int force_sig_fault_trapno(int sig, int code, void __user *addr, int trapno);
int send_sig_fault_trapno(int sig, int code, void __user *addr, int trapno,
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 928b166d888e..48ea8863183b 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6394,8 +6394,6 @@ void perf_event_wakeup(struct perf_event *event)

static void perf_sigtrap(struct perf_event *event)
{
- struct kernel_siginfo info;
-
/*
* We'd expect this to only occur if the irq_work is delayed and either
* ctx->task or current has changed in the meantime. This can be the
@@ -6410,13 +6408,8 @@ static void perf_sigtrap(struct perf_event *event)
if (current->flags & PF_EXITING)
return;

- clear_siginfo(&info);
- info.si_signo = SIGTRAP;
- info.si_code = TRAP_PERF;
- info.si_errno = event->attr.type;
- info.si_perf = event->attr.sig_data;
- info.si_addr = (void __user *)event->pending_addr;
- force_sig_info(&info);
+ force_sig_perf((void __user *)event->pending_addr,
+ event->attr.type, event->attr.sig_data);
}

static void perf_pending_event_disable(struct perf_event *event)
diff --git a/kernel/signal.c b/kernel/signal.c
index 690921960d8b..5b1ad7f080ab 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1753,6 +1753,19 @@ int force_sig_pkuerr(void __user *addr, u32 pkey)
}
#endif

+int force_sig_perf(void __user *pending_addr, u32 type, u64 sig_data)
+{
+ struct kernel_siginfo info;
+
+ clear_siginfo(&info);
+ info.si_signo = SIGTRAP;
+ info.si_errno = type;
+ info.si_code = TRAP_PERF;
+ info.si_addr = pending_addr;
+ info.si_perf = sig_data;
+ return force_sig_info(&info);
+}
+
#if IS_ENABLED(SPARC)
int force_sig_fault_trapno(int sig, int code, void __user *addr, int trapno)
{
--
2.30.1

Eric W. Biederman

unread,
Apr 30, 2021, 7:44:02 PMApr 30
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev

Don't abuse si_errno and deliver all of the perf data in si_perf.

Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
---
fs/signalfd.c | 3 ++-
include/linux/compat.h | 5 ++++-
include/uapi/asm-generic/siginfo.h | 5 ++++-
include/uapi/linux/signalfd.h | 4 ++--
kernel/signal.c | 18 +++++++++++-------
5 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/fs/signalfd.c b/fs/signalfd.c
index 83130244f653..9686af56f073 100644
--- a/fs/signalfd.c
+++ b/fs/signalfd.c
@@ -134,7 +134,8 @@ static int signalfd_copyinfo(struct signalfd_siginfo __user *uinfo,
break;
case SIL_FAULT_PERF_EVENT:
new.ssi_addr = (long) kinfo->si_addr;
- new.ssi_perf = kinfo->si_perf;
+ new.ssi_perf_type = kinfo->si_perf.type;
+ new.ssi_perf_data = kinfo->si_perf.data;
break;
case SIL_CHLD:
new.ssi_pid = kinfo->si_pid;
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 24462ed63af4..0726f9b3a57c 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -235,7 +235,10 @@ typedef struct compat_siginfo {
u32 _pkey;
} _addr_pkey;
/* used when si_code=TRAP_PERF */
- compat_ulong_t _perf;
+ struct {
+ compat_ulong_t data;
+ u32 type;
+ } _perf;
};
} _sigfault;

diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index 2abdf1d19aad..19b6310021a3 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -90,7 +90,10 @@ union __sifields {
__u32 _pkey;
} _addr_pkey;
/* used when si_code=TRAP_PERF */
- unsigned long _perf;
+ struct {
+ unsigned long data;
+ u32 type;
+ } _perf;
};
} _sigfault;

diff --git a/include/uapi/linux/signalfd.h b/include/uapi/linux/signalfd.h
index 7e333042c7e3..e78dddf433fc 100644
--- a/include/uapi/linux/signalfd.h
+++ b/include/uapi/linux/signalfd.h
@@ -39,8 +39,8 @@ struct signalfd_siginfo {
__s32 ssi_syscall;
__u64 ssi_call_addr;
__u32 ssi_arch;
- __u32 __pad3;
- __u64 ssi_perf;
+ __u32 ssi_perf_type;
+ __u64 ssi_perf_data;

/*
* Pad strcture to 128 bytes. Remember to update the
diff --git a/kernel/signal.c b/kernel/signal.c
index 5b1ad7f080ab..cb3574b7319c 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1758,11 +1758,13 @@ int force_sig_perf(void __user *pending_addr, u32 type, u64 sig_data)
struct kernel_siginfo info;

clear_siginfo(&info);
- info.si_signo = SIGTRAP;
- info.si_errno = type;
- info.si_code = TRAP_PERF;
- info.si_addr = pending_addr;
- info.si_perf = sig_data;
+ info.si_signo = SIGTRAP;
+ info.si_errno = 0;
+ info.si_code = TRAP_PERF;
+ info.si_addr = pending_addr;
+ info.si_perf.data = sig_data;
+ info.si_perf.type = type;
+
return force_sig_info(&info);
}

@@ -3379,7 +3381,8 @@ void copy_siginfo_to_external32(struct compat_siginfo *to,
break;
case SIL_FAULT_PERF_EVENT:
to->si_addr = ptr_to_compat(from->si_addr);
- to->si_perf = from->si_perf;
+ to->si_perf.data = from->si_perf.data;
+ to->si_perf.type = from->si_perf.type;
break;
case SIL_CHLD:
to->si_pid = from->si_pid;
@@ -3455,7 +3458,8 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to,
break;
case SIL_FAULT_PERF_EVENT:
to->si_addr = compat_ptr(from->si_addr);
- to->si_perf = from->si_perf;
+ to->si_perf.data = from->si_perf.data;
+ to->si_perf.type = from->si_perf.type;
break;
case SIL_CHLD:
to->si_pid = from->si_pid;
--
2.30.1

Eric W. Biederman

unread,
Apr 30, 2021, 7:48:22 PMApr 30
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev

Well with 7 patches instead of 3 that was a little more than I thought
I was going to send.

However that does demonstrate what I am thinking, and I think most of
the changes are reasonable at this point.

I am very curious how synchronous this all is, because if this code
is truly synchronous updating signalfd to handle this class of signal
doesn't really make sense.

If the code is not synchronous using force_sig is questionable.

Eric W. Biederman (7):
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 perf_data in si_perf

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/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/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 +-
fs/signalfd.c | 13 ++--
include/linux/compat.h | 9 +--
include/linux/sched/signal.h | 13 ++--
include/linux/signal.h | 3 +-
include/uapi/asm-generic/siginfo.h | 11 ++--
include/uapi/linux/signalfd.h | 4 +-
kernel/events/core.c | 11 +---
kernel/signal.c | 108 ++++++++++++++++++++++------------
23 files changed, 149 insertions(+), 142 deletions(-)

Marco Elver

unread,
Apr 30, 2021, 7:50:20 PMApr 30
to Eric W. Biederman, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
On Fri, 30 Apr 2021 at 22:15, Eric W. Biederman <ebie...@xmission.com> wrote:
[...]
> arm64 only abuses si_errno in compat code for bug compatibility with
> arm32.
>
> > Given it'd be wasted space otherwise, and we define the semantics of
> > whatever is stored in siginfo on the new signal, it'd be good to keep.
>
> Except you don't completely. You are not defining a new signal. You
> are extending the definition of SIGTRAP. Anything generic that
> responds to all SIGTRAPs can reasonably be looking at si_errno.

I see where you're coming from, and agree with this if si_errno
already had some semantics for some subset of SIGTRAPs. I've tried to
analyze the situation a bit further, since siginfo seems to be a giant
minefield and semantics is underspecified at best. :-)

Do any of the existing SIGTRAPs define si_errno to be set? As far as I
can tell, they don't.

If this is true, I think there are benefits and downsides to
repurposing si_errno (similar to what SIGSYS did). The obvious
downside is as you suggest, it's not always a real errno. The benefit
is that we avoid introducing more and more fields -- i.e. if we permit
si_errno to be repurposed for SIGTRAP and its value depends on the
precise si_code, too, we simplify siginfo's overall definition (also
given every new field needs more code in kernel/signal.c, too).

Given SIGTRAPs are in response to some user-selected event in the
user's code (breakpoints, ptrace, etc. ... now perf events), the user
must already check the si_code to select the right action because
SIGTRAPs are not alike (unlike, e.g. SIGSEGV). Because of this, I
think that repurposing si_errno in an si_code-dependent way for
SIGTRAPs is safe.

If you think it is simply untenable to repurpose si_errno for
SIGTRAPs, please confirm -- I'll just send a minimal patch to fix (I'd
probably just remove setting it... everything else is too intrusive as
a "fix".. sigh).

The cleanups as you outline below seem orthogonal and not urgent for
5.13 (all changes and cleanups for __ARCH_SI_TRAPNO seem too intrusive
without -next exposure).

Thanks,
-- Marco

Marco Elver

unread,
Apr 30, 2021, 8:28:46 PMApr 30
to Eric W. Biederman, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
Yes it's synchronous, insofar that the user will receive the signal
right when the event happens (I've tested this extensively, also see
tools/testing/selftests/perf_events). Of course, there's some effort
involved from the point where the event triggered to actually safely
delivering the signal. In particular, for HW events, these arrive in
NMI, and we can't do much in NMI, and therefore will queue an
irq_work.

On architectures that properly implement irq_work, it will do a
self-IPI, so that once it is safe to do so, another interrupt is
delivered where we process the event and do the force_sig_info(). The
task where the event occurred never got a chance to run -- except for
bad architectures with broken irq_work, and the first WARN_ON() is
there so we don't crash the kernel if somebody botched their irq_work.

Since we're talking about various HW events, these can still trigger
while the task is exiting, before perf_event_exit_task() being called
during do_exit(). That's why we have the 2nd check.

Thanks,
-- Marco

Marco Elver

unread,
Apr 30, 2021, 8:37:34 PMApr 30
to Eric W. Biederman, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
On Sat, 1 May 2021 at 01:48, Eric W. Biederman <ebie...@xmission.com> wrote:
>
> Well with 7 patches instead of 3 that was a little more than I thought
> I was going to send.
>
> However that does demonstrate what I am thinking, and I think most of
> the changes are reasonable at this point.
>
> I am very curious how synchronous this all is, because if this code
> is truly synchronous updating signalfd to handle this class of signal
> doesn't really make sense.
>
> If the code is not synchronous using force_sig is questionable.
>
> Eric W. Biederman (7):
> 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 perf_data in si_perf

Thank you for doing this so quickly -- it looks much cleaner. I'll
have a more detailed look next week and also run some tests myself.

At a first glance, you've broken our tests in
tools/testing/selftests/perf_events/ -- needs a
s/si_perf/si_perf.data/, s/si_errno/si_perf.type/

Thanks!

-- Marco

Marco Elver

unread,
May 1, 2021, 6:31:23 AMMay 1
to Eric W. Biederman, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
On Sat, 1 May 2021 at 00:50, Eric W. Biederman <ebie...@xmission.com> wrote:
>
> 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.
>

Maybe add "Link:
https://lkml.kernel.org/r/CAK8P3a0+uKYwL1NhY6Hvtieghba2hKYGD6hcKx5n8=4Gtt...@mail.gmail.com"

> Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>

Acked-by: Marco Elver <el...@google.com>

By no longer guarding it with __ARCH_SI_TRAPNO we run the risk that it
will be used by something else at some point. Is that intentional?

Thanks,
-- Marco

Marco Elver

unread,
May 1, 2021, 6:33:39 AMMay 1
to Eric W. Biederman, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
The breakage isn't apparent here, but in later patches. These need to
become CONFIG_SPARC and CONFIG_ALPHA.

Marco Elver

unread,
May 1, 2021, 6:34:06 AMMay 1
to Eric W. Biederman, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
On Sat, 1 May 2021 at 00:55, Eric W. Biederman <ebie...@xmission.com> wrote:
>
> Now that si_trapno is no longer expected to be present for every fault
> reported using siginfo on alpha and sparc remove the trapno parameter
> from force_sig_fault, force_sig_fault_to_task and send_sig_fault.
>
> Add two new helpers force_sig_fault_trapno and send_sig_fault_trapno
> for those signals where trapno is expected to be set.
>
> Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
> ---
> 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/sparc/kernel/process_64.c | 2 +-
> 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 +-
> include/linux/sched/signal.h | 12 +++------
> kernel/signal.c | 41 +++++++++++++++++++++--------
> 14 files changed, 88 insertions(+), 77 deletions(-)

This still breaks sparc64:

> sparc64-linux-gnu-ld: arch/sparc/kernel/traps_64.o: in function `bad_trap':
> (.text+0x2a4): undefined reference to `force_sig_fault_trapno'

[...]
> +#if IS_ENABLED(SPARC)

This should be 'IS_ENABLED(CONFIG_SPARC)'.

> +int force_sig_fault_trapno(int sig, int code, void __user *addr, int trapno)
> +{
> + struct kernel_siginfo info;
> +
> + clear_siginfo(&info);
> + info.si_signo = sig;
> + info.si_errno = 0;
> + info.si_code = code;
> + info.si_addr = addr;
> + info.si_trapno = trapno;
> + return force_sig_info(&info);
> +}
> +#endif
> +
> +#if IS_ENABLED(ALPHA)

CONFIG_ALPHA

Marco Elver

unread,
May 1, 2021, 6:35:41 AMMay 1
to Eric W. Biederman, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
On Sat, 1 May 2021 at 01:43, Eric W. Biederman <ebie...@xmission.com> wrote:
>
> It helps to know which part of the siginfo structure the siginfo_layout
> value is talking about.

Your Signed-off-by seems to be missing.

Otherwise,

Acked-by: Marco Elver <el...@google.com>

Marco Elver

unread,
May 1, 2021, 6:46:01 AMMay 1
to Eric W. Biederman, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
s/pending_addr/addr/

to match force_sig_perf() declaration.

Marco Elver

unread,
May 1, 2021, 6:47:33 AMMay 1
to Eric W. Biederman, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
On Sat, 1 May 2021 at 01:44, Eric W. Biederman <ebie...@xmission.com> wrote:
>
> Don't abuse si_errno and deliver all of the perf data in si_perf.
>
> Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
> ---

Thank you for the fix, this looks cleaner.

Just note that this patch needs to include updates to
tools/testing/selftests/perf_events. This should do it:
> sed -i 's/si_perf/si_perf.data/g; s/si_errno/si_perf.type/g' tools/testing/selftests/perf_events/*.c

Subject: s/perf_data/perf data/ ?

For uapi, need to switch to __u32, see below.
This needs to be __u32.

Eric W. Biederman

unread,
May 1, 2021, 11:17:06 AMMay 1
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
Yeah. I figured I did, but I couldn't figure out where the tests were
and I didn't have a lot of time. I just wanted to get this out so we
can do as much as reasonable before the ABI starts being actively used
by userspace and we can't change it.

Eric

Marco Elver

unread,
May 1, 2021, 12:24:36 PMMay 1
to Eric W. Biederman, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
No worries, and agreed. I've run tools/testing/selftests/perf_events
tests on x86-64 (native + 32-bit compat), and compile-tested x86-32,
arm64, arm (with my static asserts), m68k, and sparc64. Some trivial
breakages, note comments in other patches.

With the trivial fixes this looks good to me. I'll happily retest v2
when you send it.

Thanks,
-- Marco

Marco Elver

unread,
May 1, 2021, 12:26:29 PMMay 1
to Eric W. Biederman, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
On Sat, 1 May 2021 at 02:37, Marco Elver <el...@google.com> wrote:
> On Sat, 1 May 2021 at 01:48, Eric W. Biederman <ebie...@xmission.com> wrote:
> >
> > Well with 7 patches instead of 3 that was a little more than I thought
> > I was going to send.
> >
> > However that does demonstrate what I am thinking, and I think most of
> > the changes are reasonable at this point.
> >
> > I am very curious how synchronous this all is, because if this code
> > is truly synchronous updating signalfd to handle this class of signal
> > doesn't really make sense.

Just a note on this: the reason for adding signalfd support was based
on the comment at SIL_FAULT_PKUERR:

> /*
> * Fall through to the SIL_FAULT case. Both SIL_FAULT_BNDERR
> * and SIL_FAULT_PKUERR are only generated by faults that
> * deliver them synchronously to userspace. In case someone
> * injects one of these signals and signalfd catches it treat
> * it as SIL_FAULT.
> */

The same would hold for SIL_FAULT_PERF_EVENT, where somebody injects
(re-injects perhaps?) such an event. But otherwise, yes,
non-synchronous handling of SIGTRAP/TRAP_PERF is pretty useless for
almost all usecases I can think of.

Thanks,
-- Marco

Eric W. Biederman

unread,
May 2, 2021, 2:24:55 PMMay 2
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
Good catch. For some reason I thought IS_ENABLED added the CONFIG_
prefix but I looked and it doesn't.

Eric W. Biederman

unread,
May 2, 2021, 2:27:28 PMMay 2
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
Marco Elver <el...@google.com> writes:

> On Sat, 1 May 2021 at 00:50, Eric W. Biederman <ebie...@xmission.com> wrote:
>>
>> 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.
>>
>
> Maybe add "Link:
> https://lkml.kernel.org/r/CAK8P3a0+uKYwL1NhY6Hvtieghba2hKYGD6hcKx5n8=4Gtt...@mail.gmail.com"
>
>> Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
>
> Acked-by: Marco Elver <el...@google.com>
>
> By no longer guarding it with __ARCH_SI_TRAPNO we run the risk that it
> will be used by something else at some point. Is that intentional?

The motivation was letting the code be tested on other architectures.

But once si_trapno falls inside the union instead of being present for
every signal reporting a fault it doesn't really matter.

I think it would be poor taste but harmless to use si_trapno, mostly
because defining a new entry in the union could be more specific and
well defined.

Eric

Eric W. Biederman

unread,
May 2, 2021, 2:39:26 PMMay 2
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
Marco Elver <el...@google.com> writes:

> On Sat, 1 May 2021 at 01:44, Eric W. Biederman <ebie...@xmission.com> wrote:
>>
>> Don't abuse si_errno and deliver all of the perf data in si_perf.
>>
>> Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
>> ---
>
> Thank you for the fix, this looks cleaner.
>
> Just note that this patch needs to include updates to
> tools/testing/selftests/perf_events. This should do it:
>> sed -i 's/si_perf/si_perf.data/g; s/si_errno/si_perf.type/g' tools/testing/selftests/perf_events/*.c
>
> Subject: s/perf_data/perf data/ ?
>
> For uapi, need to switch to __u32, see below.

Good point.

The one thing that this doesn't do is give you a 64bit field
on 32bit architectures.

On 32bit builds the layout is:

int si_signo;
int si_errno;
int si_code;
void __user *_addr;

So I believe if the first 3 fields were moved into the _sifields union
si_perf could define a 64bit field as it's first member and it would not
break anything else.

Given that the data field is 64bit that seems desirable.

Eric

Marco Elver

unread,
May 2, 2021, 3:13:46 PMMay 2
to Eric W. Biederman, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
On Sun, 2 May 2021 at 20:39, Eric W. Biederman <ebie...@xmission.com> wrote:
>
> Marco Elver <el...@google.com> writes:
>
> > On Sat, 1 May 2021 at 01:44, Eric W. Biederman <ebie...@xmission.com> wrote:
> >>
> >> Don't abuse si_errno and deliver all of the perf data in si_perf.
> >>
> >> Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
> >> ---
> >
> > Thank you for the fix, this looks cleaner.
> >
> > Just note that this patch needs to include updates to
> > tools/testing/selftests/perf_events. This should do it:
> >> sed -i 's/si_perf/si_perf.data/g; s/si_errno/si_perf.type/g' tools/testing/selftests/perf_events/*.c
> >
> > Subject: s/perf_data/perf data/ ?
> >
> > For uapi, need to switch to __u32, see below.
>
> Good point.
>
> The one thing that this doesn't do is give you a 64bit field
> on 32bit architectures.
>
> On 32bit builds the layout is:
>
> int si_signo;
> int si_errno;
> int si_code;
> void __user *_addr;
>
> So I believe if the first 3 fields were moved into the _sifields union
> si_perf could define a 64bit field as it's first member and it would not
> break anything else.
>
> Given that the data field is 64bit that seems desirable.

Yes, it's quite unfortunate -- it was __u64 at first, but then we
noticed it broke 32-bit architectures like arm:
https://lore.kernel.org/linux-arch/20210422191823...@google.com/

Thanks,
-- Marco

Peter Zijlstra

unread,
May 3, 2021, 8:46:41 AMMay 3
to Eric W. Biederman, Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
On Sun, May 02, 2021 at 01:39:16PM -0500, Eric W. Biederman wrote:

> The one thing that this doesn't do is give you a 64bit field
> on 32bit architectures.
>
> On 32bit builds the layout is:
>
> int si_signo;
> int si_errno;
> int si_code;
> void __user *_addr;
>
> So I believe if the first 3 fields were moved into the _sifields union
> si_perf could define a 64bit field as it's first member and it would not
> break anything else.
>
> Given that the data field is 64bit that seems desirable.

The data field is fundamentally an address, it is internally a u64
because the perf ring buffer has u64 alignment and it saves on compat
crap etc.

So for the 32bit/compat case the high bits will always be 0 and
truncating into an unsigned long is fine.

Eric W. Biederman

unread,
May 3, 2021, 3:38:51 PMMay 3
to Peter Zijlstra, Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
I see why it is fine to truncate the data field into an unsigned long.

Other than technical difficulties in extending siginfo_t is there any
reason not to define data as a __u64?

Eric

Marco Elver

unread,
May 3, 2021, 3:53:51 PMMay 3
to Eric W. Biederman, Peter Zijlstra, Arnd Bergmann, Florian Weimer, David S. Miller, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev
No -- like I pointed at earlier, si_perf used to be __u64, but we
can't because of the siginfo_t limitation. What we have now is fine,
and not worth dwelling over given siginfo limitations.

Thanks,
-- Marco

Eric W. Biederman

unread,
May 3, 2021, 4:25:23 PMMay 3
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev

This is my attempt to sort out the ABI issues with SIGTRAP TRAP_PERF
before any userspace code starts using the new ABI.

The big ideas are:
- Placing the asserts first to prevent unexpected ABI changes
- si_trapno can become an ordinary fault subfield.
- Reworking siginfo so that si_perf_data can be a 64bit field.
- struct signalfd_siginfo is almost full

Marco I have incorporated your static_assert changes and built
on them to prevent having unexpected ABI changes.

The field si_trapno is changed to become an ordinary extension of the
_sigfault member of siginfo.

The code is refactored a bit and then si_perf_data is made a 64bit,
and si_perf_type is made distinct from si_errno.

Finally the signalfd_siginfo fields are removed as they appear to be
filling up the structure without userspace actually being able to use
them.

v1: https://lkml.kernel.org/r/m1zgxfs7...@fess.ebiederm.org

Eric W. Biederman (9):
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: Redefine signinfo so 64bit fields are possible
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 | 37 +++++++
arch/arm64/kernel/signal.c | 37 +++++++
arch/arm64/kernel/signal32.c | 37 +++++++
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 | 35 +++++++
arch/sparc/kernel/signal_64.c | 34 +++++++
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 | 20 ++--
fs/signalfd.c | 23 ++---
include/linux/compat.h | 38 ++++---
include/linux/sched/signal.h | 13 +--
include/linux/signal.h | 3 +-
include/uapi/asm-generic/siginfo.h | 57 +++++++----
include/uapi/linux/signalfd.h | 4 +-
kernel/events/core.c | 11 +-
kernel/signal.c | 113 +++++++++++++--------
.../selftests/perf_events/sigtrap_threads.c | 12 +--
30 files changed, 402 insertions(+), 191 deletions(-)

Eric W. Beiderman

unread,
May 3, 2021, 4:38:57 PMMay 3
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev, Eric W . Biederman
From: Marco Elver <el...@google.com>

To help catch ABI breaks at compile-time, add compile-time assertions to
verify the siginfo_t layout. Unlike other architectures, sparc64 is
special, because it is one of few architectures requiring si_trapno.
ABI breaks around that field would only be caught here.

Link: https://lkml.kernel.org/r/m11rat9...@fess.ebiederm.org
Suggested-by: Eric W. Biederman <ebie...@xmission.com>
Acked-by: David S. Miller <da...@davemloft.net>
Signed-off-by: Marco Elver <el...@google.com>
Signed-off-by: Eric W. Biederman <ebie...@xmission.com>
---
arch/sparc/kernel/signal32.c | 34 ++++++++++++++++++++++++++++++++++
arch/sparc/kernel/signal_64.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 67 insertions(+)

diff --git a/arch/sparc/kernel/signal32.c b/arch/sparc/kernel/signal32.c
index e9695a06492f..778ed5c26d4a 100644
--- a/arch/sparc/kernel/signal32.c
+++ b/arch/sparc/kernel/signal32.c
@@ -745,3 +745,37 @@ asmlinkage int do_sys32_sigstack(u32 u_ssptr, u32 u_ossptr, unsigned long sp)
out:
return ret;
}
+
+/*
+ * Compile-time assertions for siginfo_t offsets. Check NSIG* as well, as
+ * changes likely come with new fields that should be added below.
+ */
+static_assert(NSIGILL == 11);
+static_assert(NSIGFPE == 15);
+static_assert(NSIGSEGV == 9);
+static_assert(NSIGBUS == 5);
+static_assert(NSIGTRAP == 6);
+static_assert(NSIGCHLD == 6);
+static_assert(NSIGSYS == 2);
+static_assert(offsetof(compat_siginfo_t, si_signo) == 0x00);
+static_assert(offsetof(compat_siginfo_t, si_errno) == 0x04);
+static_assert(offsetof(compat_siginfo_t, si_code) == 0x08);
+static_assert(offsetof(compat_siginfo_t, si_pid) == 0x0c);
+static_assert(offsetof(compat_siginfo_t, si_uid) == 0x10);
+static_assert(offsetof(compat_siginfo_t, si_tid) == 0x0c);
+static_assert(offsetof(compat_siginfo_t, si_overrun) == 0x10);
+static_assert(offsetof(compat_siginfo_t, si_status) == 0x14);
+static_assert(offsetof(compat_siginfo_t, si_utime) == 0x18);
+static_assert(offsetof(compat_siginfo_t, si_stime) == 0x1c);
+static_assert(offsetof(compat_siginfo_t, si_value) == 0x14);
+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_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 a0eec62c825d..c9bbf5f29078 100644
--- a/arch/sparc/kernel/signal_64.c
+++ b/arch/sparc/kernel/signal_64.c
@@ -556,3 +556,36 @@ void do_notify_resume(struct pt_regs *regs, unsigned long orig_i0, unsigned long
user_enter();
}

+/*
+ * Compile-time assertions for siginfo_t offsets. Check NSIG* as well, as
+ * changes likely come with new fields that should be added below.
+ */
+static_assert(NSIGILL == 11);
+static_assert(NSIGFPE == 15);
+static_assert(NSIGSEGV == 9);
+static_assert(NSIGBUS == 5);
+static_assert(NSIGTRAP == 6);
+static_assert(NSIGCHLD == 6);
+static_assert(NSIGSYS == 2);
+static_assert(offsetof(siginfo_t, si_signo) == 0x00);
+static_assert(offsetof(siginfo_t, si_errno) == 0x04);
+static_assert(offsetof(siginfo_t, si_code) == 0x08);
+static_assert(offsetof(siginfo_t, si_pid) == 0x10);
+static_assert(offsetof(siginfo_t, si_uid) == 0x14);
+static_assert(offsetof(siginfo_t, si_tid) == 0x10);
+static_assert(offsetof(siginfo_t, si_overrun) == 0x14);
+static_assert(offsetof(siginfo_t, si_status) == 0x18);
+static_assert(offsetof(siginfo_t, si_utime) == 0x20);
+static_assert(offsetof(siginfo_t, si_stime) == 0x28);
+static_assert(offsetof(siginfo_t, si_value) == 0x18);
+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_band) == 0x10);
+static_assert(offsetof(siginfo_t, si_fd) == 0x14);
--
2.30.1

Eric W. Beiderman

unread,
May 3, 2021, 4:38:57 PMMay 3
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev, Eric W . Biederman
From: Marco Elver <el...@google.com>

To help catch ABI breaks at compile-time, add compile-time assertions to
verify the siginfo_t layout.

This could have caught that we cannot portably add 64-bit integers to
siginfo_t on 32-bit architectures like Arm before reaching -next:
https://lkml.kernel.org/r/20210422191823...@google.com

Signed-off-by: Marco Elver <el...@google.com>
Signed-off-by: Eric W. Biederman <ebie...@xmission.com>
---
arch/arm/kernel/signal.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index a3a38d0a4c85..2dac5d2c5cf6 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -725,3 +725,39 @@ asmlinkage void do_rseq_syscall(struct pt_regs *regs)
rseq_syscall(regs);
}
#endif
+
+/*
+ * Compile-time assertions for siginfo_t offsets. Check NSIG* as well, as
+ * changes likely come with new fields that should be added below.
+ */
+static_assert(NSIGILL == 11);
+static_assert(NSIGFPE == 15);
+static_assert(NSIGSEGV == 9);
+static_assert(NSIGBUS == 5);
+static_assert(NSIGTRAP == 6);
+static_assert(NSIGCHLD == 6);
+static_assert(NSIGSYS == 2);
+static_assert(offsetof(siginfo_t, si_signo) == 0x00);
+static_assert(offsetof(siginfo_t, si_errno) == 0x04);
+static_assert(offsetof(siginfo_t, si_code) == 0x08);
+static_assert(offsetof(siginfo_t, si_pid) == 0x0c);
+static_assert(offsetof(siginfo_t, si_uid) == 0x10);
+static_assert(offsetof(siginfo_t, si_tid) == 0x0c);
+static_assert(offsetof(siginfo_t, si_overrun) == 0x10);
+static_assert(offsetof(siginfo_t, si_status) == 0x14);
+static_assert(offsetof(siginfo_t, si_utime) == 0x18);
+static_assert(offsetof(siginfo_t, si_stime) == 0x1c);
+static_assert(offsetof(siginfo_t, si_value) == 0x14);
+static_assert(offsetof(siginfo_t, si_int) == 0x14);
+static_assert(offsetof(siginfo_t, si_ptr) == 0x14);
+static_assert(offsetof(siginfo_t, si_addr) == 0x0c);
+static_assert(offsetof(siginfo_t, si_addr_lsb) == 0x10);
+static_assert(offsetof(siginfo_t, si_lower) == 0x14);
+static_assert(offsetof(siginfo_t, si_upper) == 0x18);
+static_assert(offsetof(siginfo_t, si_pkey) == 0x14);
+static_assert(offsetof(siginfo_t, si_perf) == 0x10);
+static_assert(offsetof(siginfo_t, si_band) == 0x0c);
+static_assert(offsetof(siginfo_t, si_fd) == 0x10);
+static_assert(offsetof(siginfo_t, si_call_addr) == 0x0c);
+static_assert(offsetof(siginfo_t, si_syscall) == 0x10);
+static_assert(offsetof(siginfo_t, si_arch) == 0x14);
--
2.30.1

Eric W. Beiderman

unread,
May 3, 2021, 4:38:59 PMMay 3
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev, Eric W . Biederman
From: Marco Elver <el...@google.com>

To help catch ABI breaks at compile-time, add compile-time assertions to
verify the siginfo_t layout.

Signed-off-by: Marco Elver <el...@google.com>
Signed-off-by: Eric W. Biederman <ebie...@xmission.com>
---
arch/arm64/kernel/signal.c | 36 ++++++++++++++++++++++++++++++++++++
arch/arm64/kernel/signal32.c | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 72 insertions(+)

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 6237486ff6bb..af8bd2af1298 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -973,3 +973,39 @@ void __init minsigstksz_setup(void)
round_up(sizeof(struct frame_record), 16) +
16; /* max alignment padding */
}
+
+/*
+ * Compile-time assertions for siginfo_t offsets. Check NSIG* as well, as
+ * changes likely come with new fields that should be added below.
+ */
+static_assert(NSIGILL == 11);
+static_assert(NSIGFPE == 15);
+static_assert(NSIGSEGV == 9);
+static_assert(NSIGBUS == 5);
+static_assert(NSIGTRAP == 6);
+static_assert(NSIGCHLD == 6);
+static_assert(NSIGSYS == 2);
+static_assert(offsetof(siginfo_t, si_signo) == 0x00);
+static_assert(offsetof(siginfo_t, si_errno) == 0x04);
+static_assert(offsetof(siginfo_t, si_code) == 0x08);
+static_assert(offsetof(siginfo_t, si_pid) == 0x10);
+static_assert(offsetof(siginfo_t, si_uid) == 0x14);
+static_assert(offsetof(siginfo_t, si_tid) == 0x10);
+static_assert(offsetof(siginfo_t, si_overrun) == 0x14);
+static_assert(offsetof(siginfo_t, si_status) == 0x18);
+static_assert(offsetof(siginfo_t, si_utime) == 0x20);
+static_assert(offsetof(siginfo_t, si_stime) == 0x28);
+static_assert(offsetof(siginfo_t, si_value) == 0x18);
+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_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) == 0x18);
+static_assert(offsetof(siginfo_t, si_call_addr) == 0x10);
+static_assert(offsetof(siginfo_t, si_syscall) == 0x18);
+static_assert(offsetof(siginfo_t, si_arch) == 0x1c);
diff --git a/arch/arm64/kernel/signal32.c b/arch/arm64/kernel/signal32.c
index 2f507f565c48..b6afb646515f 100644
--- a/arch/arm64/kernel/signal32.c
+++ b/arch/arm64/kernel/signal32.c
@@ -457,3 +457,39 @@ void compat_setup_restart_syscall(struct pt_regs *regs)
{
regs->regs[7] = __NR_compat_restart_syscall;
+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);
+static_assert(offsetof(compat_siginfo_t, si_call_addr) == 0x0c);
+static_assert(offsetof(compat_siginfo_t, si_syscall) == 0x10);
+static_assert(offsetof(compat_siginfo_t, si_arch) == 0x14);
--
2.30.1

Eric W. Beiderman

unread,
May 3, 2021, 4:39:03 PMMay 3
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev, Eric W. Biederman
From: "Eric W. Biederman" <ebie...@xmission.com>

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...@mail.gmail.com
Acked-by: Marco Elver <el...@google.com>
v1: https://lkml.kernel.org/r/m1tunns7...@fess.ebiederm.org
Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
---
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 778ed5c26d4a..73fd8700df3e 100644
--- a/arch/sparc/kernel/signal32.c
+++ b/arch/sparc/kernel/signal32.c
@@ -772,10 +772,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 c9bbf5f29078..17913daa66c6 100644
--- a/arch/sparc/kernel/signal_64.c
+++ b/arch/sparc/kernel/signal_64.c
@@ -582,10 +582,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 0e5d0a7e203b..a9fcabd8a5e5 100644
--- a/arch/x86/kernel/signal_compat.c
+++ b/arch/x86/kernel/signal_compat.c
@@ -127,6 +127,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 03d6f6d2c1fe..e663bf117b46 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -63,9 +63,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 */
@@ -75,6 +72,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
@@ -150,9 +149,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

Eric W. Beiderman

unread,
May 3, 2021, 4:39:06 PMMay 3
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev, Eric W. Biederman
From: "Eric W. Biederman" <ebie...@xmission.com>

Now that si_trapno is part of the union in _si_fault and available on
all architectures, add SIL_FAULT_TRAPNO and update siginfo_layout to
return SIL_FAULT_TRAPNO when si_trapno is actually used.

Update the code that uses siginfo_layout to deal with SIL_FAULT_TRAPNO
and have the same code ignore si_trapno in in all other cases.

v1: https://lkml.kernel.org/r/m1o8dvs7...@fess.ebiederm.org
Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
---
fs/signalfd.c | 8 +++-----
include/linux/signal.h | 1 +
kernel/signal.c | 37 +++++++++++++++----------------------
3 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/fs/signalfd.c b/fs/signalfd.c
index 040a1142915f..e87e59581653 100644
--- a/fs/signalfd.c
+++ b/fs/signalfd.c
@@ -123,15 +123,13 @@ static int signalfd_copyinfo(struct signalfd_siginfo __user *uinfo,
*/
case SIL_FAULT:
new.ssi_addr = (long) kinfo->si_addr;
-#ifdef __ARCH_SI_TRAPNO
+ break;
+ case SIL_FAULT_TRAPNO:
+ new.ssi_addr = (long) kinfo->si_addr;
new.ssi_trapno = kinfo->si_trapno;
-#endif
break;
case SIL_FAULT_MCEERR:
new.ssi_addr = (long) kinfo->si_addr;
-#ifdef __ARCH_SI_TRAPNO
- new.ssi_trapno = kinfo->si_trapno;
-#endif
new.ssi_addr_lsb = (short) kinfo->si_addr_lsb;
break;
case SIL_PERF_EVENT:
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 1e98548d7cf6..5160fd45e5ca 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -40,6 +40,7 @@ enum siginfo_layout {
SIL_TIMER,
SIL_POLL,
SIL_FAULT,
+ SIL_FAULT_TRAPNO,
SIL_FAULT_MCEERR,
SIL_FAULT_BNDERR,
SIL_FAULT_PKUERR,
diff --git a/kernel/signal.c b/kernel/signal.c
index 65888aec65a0..3d3ba7949788 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1194,6 +1194,7 @@ static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
case SIL_TIMER:
case SIL_POLL:
case SIL_FAULT:
+ case SIL_FAULT_TRAPNO:
case SIL_FAULT_MCEERR:
case SIL_FAULT_BNDERR:
case SIL_FAULT_PKUERR:
@@ -2527,6 +2528,7 @@ static void hide_si_addr_tag_bits(struct ksignal *ksig)
{
switch (siginfo_layout(ksig->sig, ksig->info.si_code)) {
case SIL_FAULT:
+ case SIL_FAULT_TRAPNO:
case SIL_FAULT_MCEERR:
case SIL_FAULT_BNDERR:
case SIL_FAULT_PKUERR:
@@ -3206,6 +3208,13 @@ enum siginfo_layout siginfo_layout(unsigned sig, int si_code)
if ((sig == SIGBUS) &&
(si_code >= BUS_MCEERR_AR) && (si_code <= BUS_MCEERR_AO))
layout = SIL_FAULT_MCEERR;
+ else if (IS_ENABLED(CONFIG_ALPHA) &&
+ ((sig == SIGFPE) ||
+ ((sig == SIGTRAP) && (si_code == TRAP_UNK))))
+ layout = SIL_FAULT_TRAPNO;
+ else if (IS_ENABLED(CONFIG_SPARC) &&
+ (sig == SIGILL) && (si_code == ILL_ILLTRP))
+ layout = SIL_FAULT_TRAPNO;
else if ((sig == SIGSEGV) && (si_code == SEGV_BNDERR))
layout = SIL_FAULT_BNDERR;
#ifdef SEGV_PKUERR
@@ -3317,30 +3326,22 @@ void copy_siginfo_to_external32(struct compat_siginfo *to,
break;
case SIL_FAULT:
to->si_addr = ptr_to_compat(from->si_addr);
-#ifdef __ARCH_SI_TRAPNO
+ break;
+ case SIL_FAULT_TRAPNO:
+ to->si_addr = ptr_to_compat(from->si_addr);
to->si_trapno = from->si_trapno;
-#endif
break;
case SIL_FAULT_MCEERR:
to->si_addr = ptr_to_compat(from->si_addr);
-#ifdef __ARCH_SI_TRAPNO
- to->si_trapno = from->si_trapno;
-#endif
to->si_addr_lsb = from->si_addr_lsb;
break;
case SIL_FAULT_BNDERR:
to->si_addr = ptr_to_compat(from->si_addr);
-#ifdef __ARCH_SI_TRAPNO
- to->si_trapno = from->si_trapno;
-#endif
to->si_lower = ptr_to_compat(from->si_lower);
to->si_upper = ptr_to_compat(from->si_upper);
break;
case SIL_FAULT_PKUERR:
to->si_addr = ptr_to_compat(from->si_addr);
-#ifdef __ARCH_SI_TRAPNO
- to->si_trapno = from->si_trapno;
-#endif
to->si_pkey = from->si_pkey;
break;
case SIL_PERF_EVENT:
@@ -3401,30 +3402,22 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to,
break;
case SIL_FAULT:
to->si_addr = compat_ptr(from->si_addr);
-#ifdef __ARCH_SI_TRAPNO
+ break;
+ case SIL_FAULT_TRAPNO:
+ to->si_addr = compat_ptr(from->si_addr);
to->si_trapno = from->si_trapno;
-#endif
break;
case SIL_FAULT_MCEERR:
to->si_addr = compat_ptr(from->si_addr);
-#ifdef __ARCH_SI_TRAPNO
- to->si_trapno = from->si_trapno;
-#endif
to->si_addr_lsb = from->si_addr_lsb;
break;
case SIL_FAULT_BNDERR:
to->si_addr = compat_ptr(from->si_addr);
-#ifdef __ARCH_SI_TRAPNO
- to->si_trapno = from->si_trapno;
-#endif
to->si_lower = compat_ptr(from->si_lower);
to->si_upper = compat_ptr(from->si_upper);
break;
case SIL_FAULT_PKUERR:
to->si_addr = compat_ptr(from->si_addr);

Eric W. Beiderman

unread,
May 3, 2021, 4:39:10 PMMay 3
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev, Eric W. Biederman
From: "Eric W. Biederman" <ebie...@xmission.com>

Now that si_trapno is no longer expected to be present for every fault
reported using siginfo on alpha and sparc remove the trapno parameter
from force_sig_fault, force_sig_fault_to_task and send_sig_fault.

Add two new helpers force_sig_fault_trapno and send_sig_fautl_trapno
for those signals where trapno is expected to be set.

v1: https://lkml.kernel.org/r/m1eeers7...@fess.ebiederm.org
Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
---
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/sparc/kernel/process_64.c | 2 +-
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 +-
include/linux/sched/signal.h | 12 +++------
kernel/signal.c | 41 +++++++++++++++++++++--------
14 files changed, 88 insertions(+), 77 deletions(-)

diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
index d5367a1c6300..80c5d7fbe66a 100644
--- a/arch/alpha/kernel/osf_sys.c
+++ b/arch/alpha/kernel/osf_sys.c
@@ -878,7 +878,7 @@ SYSCALL_DEFINE5(osf_setsysinfo, unsigned long, op, void __user *, buffer,

send_sig_fault(SIGFPE, si_code,
(void __user *)NULL, /* FIXME */
- 0, current);
+ current);
}
return 0;
}
diff --git a/arch/alpha/kernel/signal.c b/arch/alpha/kernel/signal.c
index 948b89789da8..bc077babafab 100644
--- a/arch/alpha/kernel/signal.c
+++ b/arch/alpha/kernel/signal.c
@@ -219,7 +219,7 @@ do_sigreturn(struct sigcontext __user *sc)

/* Send SIGTRAP if we're single-stepping: */
if (ptrace_cancel_bpt (current)) {
- send_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *) regs->pc, 0,
+ send_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *) regs->pc,
current);
}
return;
@@ -247,7 +247,7 @@ do_rt_sigreturn(struct rt_sigframe __user *frame)

/* Send SIGTRAP if we're single-stepping: */
if (ptrace_cancel_bpt (current)) {
- send_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *) regs->pc, 0,
+ send_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *) regs->pc,
current);
}
return;
diff --git a/arch/alpha/kernel/traps.c b/arch/alpha/kernel/traps.c
index 921d4b6e4d95..0dddf9ecc1f4 100644
--- a/arch/alpha/kernel/traps.c
+++ b/arch/alpha/kernel/traps.c
@@ -227,7 +227,7 @@ do_entArith(unsigned long summary, unsigned long write_mask,
}
die_if_kernel("Arithmetic fault", regs, 0, NULL);

- send_sig_fault(SIGFPE, si_code, (void __user *) regs->pc, 0, current);
+ send_sig_fault_trapno(SIGFPE, si_code, (void __user *) regs->pc, 0, current);
}

asmlinkage void
@@ -268,12 +268,12 @@ do_entIF(unsigned long type, struct pt_regs *regs)
regs->pc -= 4; /* make pc point to former bpt */
}

- send_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->pc, 0,
+ send_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->pc,
current);
return;

case 1: /* bugcheck */
- send_sig_fault(SIGTRAP, TRAP_UNK, (void __user *) regs->pc, 0,
+ send_sig_fault(SIGTRAP, TRAP_UNK, (void __user *) regs->pc,
current);
return;

@@ -335,8 +335,8 @@ do_entIF(unsigned long type, struct pt_regs *regs)
break;
}

- send_sig_fault(signo, code, (void __user *) regs->pc, regs->r16,
- current);
+ send_sig_fault_trapno(signo, code, (void __user *) regs->pc,
+ regs->r16, current);
return;

case 4: /* opDEC */
@@ -360,9 +360,9 @@ do_entIF(unsigned long type, struct pt_regs *regs)
if (si_code == 0)
return;
if (si_code > 0) {
- send_sig_fault(SIGFPE, si_code,
- (void __user *) regs->pc, 0,
- current);
+ send_sig_fault_trapno(SIGFPE, si_code,
+ (void __user *) regs->pc,
+ 0, current);
return;
}
}
@@ -387,7 +387,7 @@ do_entIF(unsigned long type, struct pt_regs *regs)
;
}

- send_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)regs->pc, 0, current);
+ send_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)regs->pc, current);
}

/* There is an ifdef in the PALcode in MILO that enables a
@@ -402,7 +402,7 @@ do_entDbg(struct pt_regs *regs)
{
die_if_kernel("Instruction fault", regs, 0, NULL);

- force_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)regs->pc, 0);
+ force_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)regs->pc);
}


@@ -964,12 +964,12 @@ do_entUnaUser(void __user * va, unsigned long opcode,
si_code = SEGV_MAPERR;
mmap_read_unlock(mm);
}
- send_sig_fault(SIGSEGV, si_code, va, 0, current);
+ send_sig_fault(SIGSEGV, si_code, va, current);
return;

give_sigbus:
regs->pc -= 4;
- send_sig_fault(SIGBUS, BUS_ADRALN, va, 0, current);
+ send_sig_fault(SIGBUS, BUS_ADRALN, va, current);
return;
}

diff --git a/arch/alpha/mm/fault.c b/arch/alpha/mm/fault.c
index 09172f017efc..eee5102c3d88 100644
--- a/arch/alpha/mm/fault.c
+++ b/arch/alpha/mm/fault.c
@@ -219,13 +219,13 @@ do_page_fault(unsigned long address, unsigned long mmcsr,
mmap_read_unlock(mm);
/* Send a sigbus, regardless of whether we were in kernel
or user mode. */
- force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *) address, 0);
+ force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *) address);
if (!user_mode(regs))
goto no_context;
return;

do_sigsegv:
- force_sig_fault(SIGSEGV, si_code, (void __user *) address, 0);
+ force_sig_fault(SIGSEGV, si_code, (void __user *) address);
return;

#ifdef CONFIG_ALPHA_LARGE_VMALLOC
diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c
index 7afd0a859a78..29e67854d5a4 100644
--- a/arch/sparc/kernel/process_64.c
+++ b/arch/sparc/kernel/process_64.c
@@ -518,7 +518,7 @@ void synchronize_user_stack(void)

static void stack_unaligned(unsigned long sp)
{
- force_sig_fault(SIGBUS, BUS_ADRALN, (void __user *) sp, 0);
+ force_sig_fault(SIGBUS, BUS_ADRALN, (void __user *) sp);
}

static const char uwfault32[] = KERN_INFO \
diff --git a/arch/sparc/kernel/sys_sparc_32.c b/arch/sparc/kernel/sys_sparc_32.c
index be77538bc038..082a551897ed 100644
--- a/arch/sparc/kernel/sys_sparc_32.c
+++ b/arch/sparc/kernel/sys_sparc_32.c
@@ -151,7 +151,7 @@ sparc_breakpoint (struct pt_regs *regs)
#ifdef DEBUG_SPARC_BREAKPOINT
printk ("TRAP: Entering kernel PC=%x, nPC=%x\n", regs->pc, regs->npc);
#endif
- force_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->pc, 0);
+ force_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->pc);

#ifdef DEBUG_SPARC_BREAKPOINT
printk ("TRAP: Returning to space: PC=%x nPC=%x\n", regs->pc, regs->npc);
diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
index 6b92fadb6ec7..1e9a9e016237 100644
--- a/arch/sparc/kernel/sys_sparc_64.c
+++ b/arch/sparc/kernel/sys_sparc_64.c
@@ -514,7 +514,7 @@ asmlinkage void sparc_breakpoint(struct pt_regs *regs)
#ifdef DEBUG_SPARC_BREAKPOINT
printk ("TRAP: Entering kernel PC=%lx, nPC=%lx\n", regs->tpc, regs->tnpc);
#endif
- force_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->tpc, 0);
+ force_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->tpc);
#ifdef DEBUG_SPARC_BREAKPOINT
printk ("TRAP: Returning to space: PC=%lx nPC=%lx\n", regs->tpc, regs->tnpc);
#endif
diff --git a/arch/sparc/kernel/traps_32.c b/arch/sparc/kernel/traps_32.c
index 247a0d9683b2..5630e5a395e0 100644
--- a/arch/sparc/kernel/traps_32.c
+++ b/arch/sparc/kernel/traps_32.c
@@ -102,8 +102,8 @@ void do_hw_interrupt(struct pt_regs *regs, unsigned long type)
if(regs->psr & PSR_PS)
die_if_kernel("Kernel bad trap", regs);

- force_sig_fault(SIGILL, ILL_ILLTRP,
- (void __user *)regs->pc, type - 0x80);
+ force_sig_fault_trapno(SIGILL, ILL_ILLTRP,
+ (void __user *)regs->pc, type - 0x80);
}

void do_illegal_instruction(struct pt_regs *regs, unsigned long pc, unsigned long npc,
@@ -116,7 +116,7 @@ void do_illegal_instruction(struct pt_regs *regs, unsigned long pc, unsigned lon
regs->pc, *(unsigned long *)regs->pc);
#endif

- send_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)pc, 0, current);
+ send_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)pc, current);
}

void do_priv_instruction(struct pt_regs *regs, unsigned long pc, unsigned long npc,
@@ -124,7 +124,7 @@ void do_priv_instruction(struct pt_regs *regs, unsigned long pc, unsigned long n
{
if(psr & PSR_PS)
die_if_kernel("Penguin instruction from Penguin mode??!?!", regs);
- send_sig_fault(SIGILL, ILL_PRVOPC, (void __user *)pc, 0, current);
+ send_sig_fault(SIGILL, ILL_PRVOPC, (void __user *)pc, current);
}

/* XXX User may want to be allowed to do this. XXX */
@@ -145,7 +145,7 @@ void do_memaccess_unaligned(struct pt_regs *regs, unsigned long pc, unsigned lon
#endif
send_sig_fault(SIGBUS, BUS_ADRALN,
/* FIXME: Should dig out mna address */ (void *)0,
- 0, current);
+ current);
}

static unsigned long init_fsr = 0x0UL;
@@ -291,7 +291,7 @@ void do_fpe_trap(struct pt_regs *regs, unsigned long pc, unsigned long npc,
else if (fsr & 0x01)
code = FPE_FLTRES;
}
- send_sig_fault(SIGFPE, code, (void __user *)pc, 0, fpt);
+ send_sig_fault(SIGFPE, code, (void __user *)pc, fpt);
#ifndef CONFIG_SMP
last_task_used_math = NULL;
#endif
@@ -305,7 +305,7 @@ void handle_tag_overflow(struct pt_regs *regs, unsigned long pc, unsigned long n
{
if(psr & PSR_PS)
die_if_kernel("Penguin overflow trap from kernel mode", regs);
- send_sig_fault(SIGEMT, EMT_TAGOVF, (void __user *)pc, 0, current);
+ send_sig_fault(SIGEMT, EMT_TAGOVF, (void __user *)pc, current);
}

void handle_watchpoint(struct pt_regs *regs, unsigned long pc, unsigned long npc,
@@ -327,13 +327,13 @@ void handle_reg_access(struct pt_regs *regs, unsigned long pc, unsigned long npc
printk("Register Access Exception at PC %08lx NPC %08lx PSR %08lx\n",
pc, npc, psr);
#endif
- force_sig_fault(SIGBUS, BUS_OBJERR, (void __user *)pc, 0);
+ force_sig_fault(SIGBUS, BUS_OBJERR, (void __user *)pc);
}

void handle_cp_disabled(struct pt_regs *regs, unsigned long pc, unsigned long npc,
unsigned long psr)
{
- send_sig_fault(SIGILL, ILL_COPROC, (void __user *)pc, 0, current);
+ send_sig_fault(SIGILL, ILL_COPROC, (void __user *)pc, current);
}

void handle_cp_exception(struct pt_regs *regs, unsigned long pc, unsigned long npc,
@@ -343,13 +343,13 @@ void handle_cp_exception(struct pt_regs *regs, unsigned long pc, unsigned long n
printk("Co-Processor Exception at PC %08lx NPC %08lx PSR %08lx\n",
pc, npc, psr);
#endif
- send_sig_fault(SIGILL, ILL_COPROC, (void __user *)pc, 0, current);
+ send_sig_fault(SIGILL, ILL_COPROC, (void __user *)pc, current);
}

void handle_hw_divzero(struct pt_regs *regs, unsigned long pc, unsigned long npc,
unsigned long psr)
{
- send_sig_fault(SIGFPE, FPE_INTDIV, (void __user *)pc, 0, current);
+ send_sig_fault(SIGFPE, FPE_INTDIV, (void __user *)pc, current);
}

#ifdef CONFIG_DEBUG_BUGVERBOSE
diff --git a/arch/sparc/kernel/traps_64.c b/arch/sparc/kernel/traps_64.c
index a850dccd78ea..6863025ed56d 100644
--- a/arch/sparc/kernel/traps_64.c
+++ b/arch/sparc/kernel/traps_64.c
@@ -107,8 +107,8 @@ void bad_trap(struct pt_regs *regs, long lvl)
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGILL, ILL_ILLTRP,
- (void __user *)regs->tpc, lvl);
+ force_sig_fault_trapno(SIGILL, ILL_ILLTRP,
+ (void __user *)regs->tpc, lvl);
}

void bad_trap_tl1(struct pt_regs *regs, long lvl)
@@ -201,8 +201,7 @@ void spitfire_insn_access_exception(struct pt_regs *regs, unsigned long sfsr, un
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGSEGV, SEGV_MAPERR,
- (void __user *)regs->tpc, 0);
+ force_sig_fault(SIGSEGV, SEGV_MAPERR, (void __user *)regs->tpc);
out:
exception_exit(prev_state);
}
@@ -237,7 +236,7 @@ void sun4v_insn_access_exception(struct pt_regs *regs, unsigned long addr, unsig
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGSEGV, SEGV_MAPERR, (void __user *) addr, 0);
+ force_sig_fault(SIGSEGV, SEGV_MAPERR, (void __user *) addr);
}

void sun4v_insn_access_exception_tl1(struct pt_regs *regs, unsigned long addr, unsigned long type_ctx)
@@ -321,7 +320,7 @@ void spitfire_data_access_exception(struct pt_regs *regs, unsigned long sfsr, un
if (is_no_fault_exception(regs))
return;

- force_sig_fault(SIGSEGV, SEGV_MAPERR, (void __user *)sfar, 0);
+ force_sig_fault(SIGSEGV, SEGV_MAPERR, (void __user *)sfar);
out:
exception_exit(prev_state);
}
@@ -385,13 +384,13 @@ void sun4v_data_access_exception(struct pt_regs *regs, unsigned long addr, unsig
*/
switch (type) {
case HV_FAULT_TYPE_INV_ASI:
- force_sig_fault(SIGILL, ILL_ILLADR, (void __user *)addr, 0);
+ force_sig_fault(SIGILL, ILL_ILLADR, (void __user *)addr);
break;
case HV_FAULT_TYPE_MCD_DIS:
- force_sig_fault(SIGSEGV, SEGV_ACCADI, (void __user *)addr, 0);
+ force_sig_fault(SIGSEGV, SEGV_ACCADI, (void __user *)addr);
break;
default:
- force_sig_fault(SIGSEGV, SEGV_MAPERR, (void __user *)addr, 0);
+ force_sig_fault(SIGSEGV, SEGV_MAPERR, (void __user *)addr);
break;
}
}
@@ -568,7 +567,7 @@ static void spitfire_ue_log(unsigned long afsr, unsigned long afar, unsigned lon
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGBUS, BUS_OBJERR, (void *)0, 0);
+ force_sig_fault(SIGBUS, BUS_OBJERR, (void *)0);
}

void spitfire_access_error(struct pt_regs *regs, unsigned long status_encoded, unsigned long afar)
@@ -2069,8 +2068,7 @@ void do_mcd_err(struct pt_regs *regs, struct sun4v_error_entry ent)
/* Send SIGSEGV to the userspace process with the right signal
* code
*/
- force_sig_fault(SIGSEGV, SEGV_ADIDERR, (void __user *)ent.err_raddr,
- 0);
+ force_sig_fault(SIGSEGV, SEGV_ADIDERR, (void __user *)ent.err_raddr);
}

/* We run with %pil set to PIL_NORMAL_MAX and PSTATE_IE enabled in %pstate.
@@ -2184,7 +2182,7 @@ bool sun4v_nonresum_error_user_handled(struct pt_regs *regs,
}
if (attrs & SUN4V_ERR_ATTRS_PIO) {
force_sig_fault(SIGBUS, BUS_ADRERR,
- (void __user *)sun4v_get_vaddr(regs), 0);
+ (void __user *)sun4v_get_vaddr(regs));
return true;
}

@@ -2340,8 +2338,7 @@ static void do_fpe_common(struct pt_regs *regs)
else if (fsr & 0x01)
code = FPE_FLTRES;
}
- force_sig_fault(SIGFPE, code,
- (void __user *)regs->tpc, 0);
+ force_sig_fault(SIGFPE, code, (void __user *)regs->tpc);
}
}

@@ -2395,8 +2392,7 @@ void do_tof(struct pt_regs *regs)
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGEMT, EMT_TAGOVF,
- (void __user *)regs->tpc, 0);
+ force_sig_fault(SIGEMT, EMT_TAGOVF, (void __user *)regs->tpc);
out:
exception_exit(prev_state);
}
@@ -2415,8 +2411,7 @@ void do_div0(struct pt_regs *regs)
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGFPE, FPE_INTDIV,
- (void __user *)regs->tpc, 0);
+ force_sig_fault(SIGFPE, FPE_INTDIV, (void __user *)regs->tpc);
out:
exception_exit(prev_state);
}
@@ -2612,7 +2607,7 @@ void do_illegal_instruction(struct pt_regs *regs)
}
}
}
- force_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)pc, 0);
+ force_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)pc);
out:
exception_exit(prev_state);
}
@@ -2632,7 +2627,7 @@ void mem_address_unaligned(struct pt_regs *regs, unsigned long sfar, unsigned lo
if (is_no_fault_exception(regs))
return;

- force_sig_fault(SIGBUS, BUS_ADRALN, (void __user *)sfar, 0);
+ force_sig_fault(SIGBUS, BUS_ADRALN, (void __user *)sfar);
out:
exception_exit(prev_state);
}
@@ -2650,7 +2645,7 @@ void sun4v_do_mna(struct pt_regs *regs, unsigned long addr, unsigned long type_c
if (is_no_fault_exception(regs))
return;

- force_sig_fault(SIGBUS, BUS_ADRALN, (void __user *) addr, 0);
+ force_sig_fault(SIGBUS, BUS_ADRALN, (void __user *) addr);
}

/* sun4v_mem_corrupt_detect_precise() - Handle precise exception on an ADI
@@ -2697,7 +2692,7 @@ void sun4v_mem_corrupt_detect_precise(struct pt_regs *regs, unsigned long addr,
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGSEGV, SEGV_ADIPERR, (void __user *)addr, 0);
+ force_sig_fault(SIGSEGV, SEGV_ADIPERR, (void __user *)addr);
}

void do_privop(struct pt_regs *regs)
@@ -2712,8 +2707,7 @@ void do_privop(struct pt_regs *regs)
regs->tpc &= 0xffffffff;
regs->tnpc &= 0xffffffff;
}
- force_sig_fault(SIGILL, ILL_PRVOPC,
- (void __user *)regs->tpc, 0);
+ force_sig_fault(SIGILL, ILL_PRVOPC, (void __user *)regs->tpc);
out:
exception_exit(prev_state);
}
diff --git a/arch/sparc/kernel/unaligned_32.c b/arch/sparc/kernel/unaligned_32.c
index ef5c5207c9ff..455f0258c745 100644
--- a/arch/sparc/kernel/unaligned_32.c
+++ b/arch/sparc/kernel/unaligned_32.c
@@ -278,5 +278,5 @@ asmlinkage void user_unaligned_trap(struct pt_regs *regs, unsigned int insn)
{
send_sig_fault(SIGBUS, BUS_ADRALN,
(void __user *)safe_compute_effective_address(regs, insn),
- 0, current);
+ current);
}
diff --git a/arch/sparc/mm/fault_32.c b/arch/sparc/mm/fault_32.c
index de2031c2b2d7..fa858626b85b 100644
--- a/arch/sparc/mm/fault_32.c
+++ b/arch/sparc/mm/fault_32.c
@@ -83,7 +83,7 @@ static void __do_fault_siginfo(int code, int sig, struct pt_regs *regs,
show_signal_msg(regs, sig, code,
addr, current);

- force_sig_fault(sig, code, (void __user *) addr, 0);
+ force_sig_fault(sig, code, (void __user *) addr);
}

static unsigned long compute_si_addr(struct pt_regs *regs, int text_fault)
diff --git a/arch/sparc/mm/fault_64.c b/arch/sparc/mm/fault_64.c
index 0a6bcc85fba7..9a9652a15fed 100644
--- a/arch/sparc/mm/fault_64.c
+++ b/arch/sparc/mm/fault_64.c
@@ -176,7 +176,7 @@ static void do_fault_siginfo(int code, int sig, struct pt_regs *regs,
if (unlikely(show_unhandled_signals))
show_signal_msg(regs, sig, code, addr, current);

- force_sig_fault(sig, code, (void __user *) addr, 0);
+ force_sig_fault(sig, code, (void __user *) addr);
}

static unsigned int get_fault_insn(struct pt_regs *regs, unsigned int insn)
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 3f6a0fcaa10c..7daa425f3055 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -298,11 +298,6 @@ static inline void kernel_signal_stop(void)

schedule();
}
-#ifdef __ARCH_SI_TRAPNO
-# define ___ARCH_SI_TRAPNO(_a1) , _a1
-#else
-# define ___ARCH_SI_TRAPNO(_a1)
-#endif
#ifdef __ia64__
# define ___ARCH_SI_IA64(_a1, _a2, _a3) , _a1, _a2, _a3
#else
@@ -310,14 +305,11 @@ static inline void kernel_signal_stop(void)
#endif

int force_sig_fault_to_task(int sig, int code, void __user *addr
- ___ARCH_SI_TRAPNO(int trapno)
___ARCH_SI_IA64(int imm, unsigned int flags, unsigned long isr)
, struct task_struct *t);
int force_sig_fault(int sig, int code, void __user *addr
- ___ARCH_SI_TRAPNO(int trapno)
___ARCH_SI_IA64(int imm, unsigned int flags, unsigned long isr));
int send_sig_fault(int sig, int code, void __user *addr
- ___ARCH_SI_TRAPNO(int trapno)
___ARCH_SI_IA64(int imm, unsigned int flags, unsigned long isr)
, struct task_struct *t);

@@ -327,6 +319,10 @@ int send_sig_mceerr(int code, void __user *, short, struct task_struct *);
int force_sig_bnderr(void __user *addr, void __user *lower, void __user *upper);
int force_sig_pkuerr(void __user *addr, u32 pkey);

+int force_sig_fault_trapno(int sig, int code, void __user *addr, int trapno);
+int send_sig_fault_trapno(int sig, int code, void __user *addr, int trapno,
+ struct task_struct *task);
+
int force_sig_ptrace_errno_trap(int errno, void __user *addr);

extern int send_sig_info(int, struct kernel_siginfo *, struct task_struct *);
diff --git a/kernel/signal.c b/kernel/signal.c
index 3d3ba7949788..7eaa8d84db4c 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1651,7 +1651,6 @@ void force_sigsegv(int sig)
}

int force_sig_fault_to_task(int sig, int code, void __user *addr
- ___ARCH_SI_TRAPNO(int trapno)
___ARCH_SI_IA64(int imm, unsigned int flags, unsigned long isr)
, struct task_struct *t)
{
@@ -1662,9 +1661,6 @@ int force_sig_fault_to_task(int sig, int code, void __user *addr
info.si_errno = 0;
info.si_code = code;
info.si_addr = addr;
-#ifdef __ARCH_SI_TRAPNO
- info.si_trapno = trapno;
-#endif
#ifdef __ia64__
info.si_imm = imm;
info.si_flags = flags;
@@ -1674,16 +1670,13 @@ int force_sig_fault_to_task(int sig, int code, void __user *addr
}

int force_sig_fault(int sig, int code, void __user *addr
- ___ARCH_SI_TRAPNO(int trapno)
___ARCH_SI_IA64(int imm, unsigned int flags, unsigned long isr))
{
return force_sig_fault_to_task(sig, code, addr
- ___ARCH_SI_TRAPNO(trapno)
___ARCH_SI_IA64(imm, flags, isr), current);
}

int send_sig_fault(int sig, int code, void __user *addr
- ___ARCH_SI_TRAPNO(int trapno)
___ARCH_SI_IA64(int imm, unsigned int flags, unsigned long isr)
, struct task_struct *t)
{
@@ -1694,9 +1687,6 @@ int send_sig_fault(int sig, int code, void __user *addr
info.si_errno = 0;
info.si_code = code;
info.si_addr = addr;
-#ifdef __ARCH_SI_TRAPNO
- info.si_trapno = trapno;
-#endif
#ifdef __ia64__
info.si_imm = imm;
info.si_flags = flags;
@@ -1763,6 +1753,37 @@ int force_sig_pkuerr(void __user *addr, u32 pkey)
}
#endif

+#if IS_ENABLED(CONFIG_SPARC)
+int force_sig_fault_trapno(int sig, int code, void __user *addr, int trapno)
+{
+ struct kernel_siginfo info;
+
+ clear_siginfo(&info);
+ info.si_signo = sig;
+ info.si_errno = 0;
+ info.si_code = code;
+ info.si_addr = addr;
+ info.si_trapno = trapno;
+ return force_sig_info(&info);
+}
+#endif
+
+#if IS_ENABLED(CONFIG_ALPHA)
+int send_sig_fault_trapno(int sig, int code, void __user *addr, int trapno,
+ struct task_struct *t)
+{
+ struct kernel_siginfo info;
+
+ clear_siginfo(&info);
+ info.si_signo = sig;
+ info.si_errno = 0;

Eric W. Beiderman

unread,
May 3, 2021, 4:39:14 PMMay 3
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev, Eric W. Biederman
From: "Eric W. Biederman" <ebie...@xmission.com>

Now that this define is no longer used remove it from the kernel.

v1: https://lkml.kernel.org/r/m18s4zs7...@fess.ebiederm.org
Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
---
arch/alpha/include/uapi/asm/siginfo.h | 2 --
arch/mips/include/uapi/asm/siginfo.h | 2 --
arch/sparc/include/uapi/asm/siginfo.h | 3 ---
3 files changed, 7 deletions(-)

diff --git a/arch/alpha/include/uapi/asm/siginfo.h b/arch/alpha/include/uapi/asm/siginfo.h
index 6e1a2af2f962..e08eae88182b 100644
--- a/arch/alpha/include/uapi/asm/siginfo.h
+++ b/arch/alpha/include/uapi/asm/siginfo.h
@@ -2,8 +2,6 @@
#ifndef _ALPHA_SIGINFO_H
#define _ALPHA_SIGINFO_H

-#define __ARCH_SI_TRAPNO
-
#include <asm-generic/siginfo.h>

#endif
diff --git a/arch/mips/include/uapi/asm/siginfo.h b/arch/mips/include/uapi/asm/siginfo.h
index c34c7eef0a1c..8cb8bd061a68 100644
--- a/arch/mips/include/uapi/asm/siginfo.h
+++ b/arch/mips/include/uapi/asm/siginfo.h
@@ -10,9 +10,7 @@
#ifndef _UAPI_ASM_SIGINFO_H
#define _UAPI_ASM_SIGINFO_H

-
#define __ARCH_SIGEV_PREAMBLE_SIZE (sizeof(long) + 2*sizeof(int))
-#undef __ARCH_SI_TRAPNO /* exception code needs to fill this ... */

#define __ARCH_HAS_SWAPPED_SIGINFO

diff --git a/arch/sparc/include/uapi/asm/siginfo.h b/arch/sparc/include/uapi/asm/siginfo.h
index 68bdde4c2a2e..0e7c27522aed 100644
--- a/arch/sparc/include/uapi/asm/siginfo.h
+++ b/arch/sparc/include/uapi/asm/siginfo.h
@@ -8,9 +8,6 @@

#endif /* defined(__sparc__) && defined(__arch64__) */

-
-#define __ARCH_SI_TRAPNO
-
#include <asm-generic/siginfo.h>


--
2.30.1

Eric W. Beiderman

unread,
May 3, 2021, 4:39:16 PMMay 3
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev, Eric W. Biederman
From: "Eric W. Biederman" <ebie...@xmission.com>

It helps to know which part of the siginfo structure the siginfo_layout
value is talking about.

v1: https://lkml.kernel.org/r/m18s4zs7...@fess.ebiederm.org
Acked-by: Marco Elver <el...@google.com>
Signed-off-by: Eric W. Biederman <ebie...@xmission.com>
---
fs/signalfd.c | 2 +-
include/linux/signal.h | 2 +-
kernel/signal.c | 10 +++++-----
3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/signalfd.c b/fs/signalfd.c
index e87e59581653..83130244f653 100644
--- a/fs/signalfd.c
+++ b/fs/signalfd.c
@@ -132,7 +132,7 @@ static int signalfd_copyinfo(struct signalfd_siginfo __user *uinfo,
new.ssi_addr = (long) kinfo->si_addr;
new.ssi_addr_lsb = (short) kinfo->si_addr_lsb;
break;
- case SIL_PERF_EVENT:
+ case SIL_FAULT_PERF_EVENT:
new.ssi_addr = (long) kinfo->si_addr;
new.ssi_perf = kinfo->si_perf;
break;
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 5160fd45e5ca..ed896d790e46 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -44,7 +44,7 @@ enum siginfo_layout {
SIL_FAULT_MCEERR,
SIL_FAULT_BNDERR,
SIL_FAULT_PKUERR,
- SIL_PERF_EVENT,
+ SIL_FAULT_PERF_EVENT,
SIL_CHLD,
SIL_RT,
SIL_SYS,
diff --git a/kernel/signal.c b/kernel/signal.c
index 7eaa8d84db4c..697c5fe58db8 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1198,7 +1198,7 @@ static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
case SIL_FAULT_MCEERR:
case SIL_FAULT_BNDERR:
case SIL_FAULT_PKUERR:
- case SIL_PERF_EVENT:
+ case SIL_FAULT_PERF_EVENT:
case SIL_SYS:
ret = false;
break;
@@ -2553,7 +2553,7 @@ static void hide_si_addr_tag_bits(struct ksignal *ksig)
case SIL_FAULT_MCEERR:
case SIL_FAULT_BNDERR:
case SIL_FAULT_PKUERR:
- case SIL_PERF_EVENT:
+ case SIL_FAULT_PERF_EVENT:
ksig->info.si_addr = arch_untagged_si_addr(
ksig->info.si_addr, ksig->sig, ksig->info.si_code);
break;
@@ -3243,7 +3243,7 @@ enum siginfo_layout siginfo_layout(unsigned sig, int si_code)
layout = SIL_FAULT_PKUERR;
#endif
else if ((sig == SIGTRAP) && (si_code == TRAP_PERF))
- layout = SIL_PERF_EVENT;
+ layout = SIL_FAULT_PERF_EVENT;
}
else if (si_code <= NSIGPOLL)
layout = SIL_POLL;
@@ -3365,7 +3365,7 @@ void copy_siginfo_to_external32(struct compat_siginfo *to,
to->si_addr = ptr_to_compat(from->si_addr);
to->si_pkey = from->si_pkey;
break;
- case SIL_PERF_EVENT:
+ case SIL_FAULT_PERF_EVENT:
to->si_addr = ptr_to_compat(from->si_addr);
to->si_perf = from->si_perf;
break;
@@ -3441,7 +3441,7 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to,
to->si_addr = compat_ptr(from->si_addr);
to->si_pkey = from->si_pkey;
break;
- case SIL_PERF_EVENT:
+ case SIL_FAULT_PERF_EVENT:
to->si_addr = compat_ptr(from->si_addr);

Eric W. Beiderman

unread,
May 3, 2021, 4:39:19 PMMay 3
to Marco Elver, Arnd Bergmann, Florian Weimer, David S. Miller, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Peter Collingbourne, Dmitry Vyukov, Alexander Potapenko, sparclinux, linux-arch, Linux Kernel Mailing List, Linux API, kasan-dev, Eric W. Biederman
From: "Eric W. Biederman" <ebie...@xmission.com>

Separate generating the signal from deciding it needs to be sent.

v1: https://lkml.kernel.org/r/m17dkjqq...@fess.ebiederm.org
Signed-off-by: "Eric W. Biederman" <ebie...@xmission.com>
---
include/linux/sched/signal.h | 1 +
kernel/events/core.c | 11 ++---------
kernel/signal.c | 13 +++++++++++++
3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 7daa425f3055..1e2f61a1a512 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -318,6 +318,7 @@ int send_sig_mceerr(int code, void __user *, short, struct task_struct *);

int force_sig_bnderr(void __user *addr, void __user *lower, void __user *upper);
int force_sig_pkuerr(void __user *addr, u32 pkey);
+int force_sig_perf(void __user *addr, u32 type, u64 sig_data);

int force_sig_fault_trapno(int sig, int code, void __user *addr, int trapno);
int send_sig_fault_trapno(int sig, int code, void __user *addr, int trapno,
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 928b166d888e..48ea8863183b 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6394,8 +6394,6 @@ void perf_event_wakeup(struct perf_event *event)

static void perf_sigtrap(struct perf_event *event)
{
- struct kernel_siginfo info;
-
/*
* We'd expect this to only occur if the irq_work is delayed and either
* ctx->task or current has changed in the meantime. This can be the
@@ -6410,13 +6408,8 @@ static void perf_sigtrap(struct perf_event *event)
if (current->flags & PF_EXITING)
return;

- clear_siginfo(&info);
- info.si_signo = SIGTRAP;
- info.si_code = TRAP_PERF;
- info.si_errno = event->attr.type;
- info.si_perf = event->attr.sig_data;
- info.si_addr = (void __user *)event->pending_addr;
- force_sig_info(&info);
+ force_sig_perf((void __user *)event->pending_addr,
+ event->attr.type, event->attr.sig_data);
}

static void perf_pending_event_disable(struct perf_event *event)
diff --git a/kernel/signal.c b/kernel/signal.c
index 697c5fe58db8..49560ceac048 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1753,6 +1753,19 @@ int force_sig_pkuerr(void __user *addr, u32 pkey)
}
#endif

+int force_sig_perf(void __user *addr, u32 type, u64 sig_data)
+{
+ struct kernel_siginfo info;
+
+ clear_siginfo(&info);