| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 21 Apr 2021 20:23:43 +0200
From: Marco Elver <elver@...gle.com>
To: Marek Szyprowski <m.szyprowski@...sung.com>
Cc: Peter Zijlstra <peterz@...radead.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Ingo Molnar <mingo@...hat.com>, Jiri Olsa <jolsa@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Namhyung Kim <namhyung@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Alexander Potapenko <glider@...gle.com>,
Al Viro <viro@...iv.linux.org.uk>,
Arnd Bergmann <arnd@...db.de>,
Christian Brauner <christian@...uner.io>,
Dmitry Vyukov <dvyukov@...gle.com>,
Jann Horn <jannh@...gle.com>, Jens Axboe <axboe@...nel.dk>,
Matt Morehouse <mascasa@...gle.com>,
Peter Collingbourne <pcc@...gle.com>,
Ian Rogers <irogers@...gle.com>,
Oleg Nesterov <oleg@...hat.com>,
kasan-dev <kasan-dev@...glegroups.com>,
linux-arch <linux-arch@...r.kernel.org>,
linux-fsdevel <linux-fsdevel@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
the arch/x86 maintainers <x86@...nel.org>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@...r.kernel.org>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
linux-tegra@...r.kernel.org, jonathanh@...dia.com
Subject: Re: [PATCH v4 05/10] signal: Introduce TRAP_PERF si_code and si_perf
to siginfo
On Wed, Apr 21, 2021 at 06:27PM +0200, Marco Elver wrote:
> 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?
So I think we just have to settle on 'unsigned long' here. On many
architectures, like 32-bit Arm, the alignment of a structure is that of
its largest member. This means that there is no portable way to add
64-bit integers to siginfo_t on 32-bit architectures.
In the case of the si_perf field, word size is sufficient since the data
it contains is user-defined. On 32-bit architectures, any excess bits of
perf_event_attr::sig_data will therefore be truncated when copying into
si_perf.
Feel free to test the below if you have time, but the below lets me boot
32-bit arm which previously timed out. It also passes all the
static_asserts() I added (will send those as separate patches).
Once I'm convinced this passes all others tests too, I'll send a patch.
Thanks,
-- Marco
diff --git a/include/linux/compat.h b/include/linux/compat.h
index c8821d966812..f0d2dd35d408 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -237,7 +237,7 @@ typedef struct compat_siginfo {
u32 _pkey;
} _addr_pkey;
/* used when si_code=TRAP_PERF */
- compat_u64 _perf;
+ compat_ulong_t _perf;
};
} _sigfault;
diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index d0bb9125c853..03d6f6d2c1fe 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -92,7 +92,7 @@ union __sifields {
__u32 _pkey;
} _addr_pkey;
/* used when si_code=TRAP_PERF */
- __u64 _perf;
+ unsigned long _perf;
};
} _sigfault;
Powered by blists - more mailing lists