Re: [PATCH v4 05/10] signal: Introduce TRAP_PERF si_code and si_perf to siginfo

[Marco Elver <elver@google.com>]

On Wed, Apr 21, 2021 at 05:11PM +0200, Marco Elver wrote:
> +Cc linux-arm-kernel
> 
[...]
> >
> > I've managed to reproduce this issue with a public Raspberry Pi OS Lite
> > rootfs image, even without deploying kernel modules:
> >
> > https://downloads.raspberrypi.org/raspios_lite_armhf/images/raspios_lite_armhf-2021-03-25/2021-03-04-raspios-buster-armhf-lite.zip
> >
> > # qemu-system-arm -M virt -smp 2 -m 512 -kernel zImage -append "earlycon
> > console=ttyAMA0 root=/dev/vda2 rw rootwait" -serial stdio -display none
> > -monitor null -device virtio-blk-device,drive=virtio-blk -drive
> > file=/tmp/2021-03-04-raspios-buster-armhf-lite.img,id=virtio-blk,if=none,format=raw
> > -netdev user,id=user -device virtio-net-device,netdev=user
> >
> > The above one doesn't boot if zImage z compiled from commit fb6cc127e0b6
> > and boots if compiled from 2e498d0a74e5. In both cases I've used default
> > arm/multi_v7_defconfig and
> > gcc-linaro-6.4.1-2017.11-x86_64_arm-linux-gnueabi toolchain.
> 
> Yup, I've narrowed it down to the addition of "__u64 _perf" to
> siginfo_t. My guess is the __u64 causes a different alignment for a
> bunch of adjacent fields. It seems that x86 and m68k are the only ones
> that have compile-time tests for the offsets. Arm should probably add
> those -- I have added a bucket of static_assert() in
> arch/arm/kernel/signal.c and see that something's off.
> 
> I'll hopefully have a fix in a day or so.

Arm and compiler folks: are there some special alignment requirement for
__u64 on arm 32-bit? (And if there is for arm64, please shout as well.)

With the static-asserts below, the only thing that I can do to fix it is
to completely remove the __u64. Padding it before or after with __u32
just does not work. It seems that the use of __u64 shifts everything
in __sifields by 4 bytes.

diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index d0bb9125c853..b02a4ac55938 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -92,7 +92,10 @@ union __sifields {
 				__u32 _pkey;
 			} _addr_pkey;
 			/* used when si_code=TRAP_PERF */
-			__u64 _perf;
+			struct {
+				__u32 _perf1;
+				__u32 _perf2;
+			} _perf;
 		};
 	} _sigfault;

^^ works, but I'd hate to have to split this into 2 __u32 because it
makes the whole design worse.

What alignment trick do we have to do here to fix it for __u64?


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

diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index a3a38d0a4c85..6c558dc314c3 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -725,3 +725,41 @@ asmlinkage void do_rseq_syscall(struct pt_regs *regs)
 	rseq_syscall(regs);
 }
 #endif
+
+/*
+ * Compile-time tests for siginfo_t offsets. Changes to NSIG* likely come with
+ * new fields; new fields 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);
+#if 0
+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);
+#endif