Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy

[Ian Rogers]

On Mon, Oct 19, 2026 at 9:28 AM Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> On Mon, Oct 19, 2026, at 17:10, Ian Rogers wrote:
> > On Mon, Oct 19, 2026 at 5:37 AM Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote:
> >> On 10.10.2026 08:04:42, Ian Rogers wrote:
> >
> > It is a little more manual than automagic :-) When perf builds there
> > is a script that complains if the tools' version of files is
> > out-of-sync with the kernel's:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/check-headers.sh#n28
> > It doesn't break the perf build, it just adds noise to it.
>
> I see that the include/linux/unaligned.h and tools/include/linux/unaligned.h
> headers have not been in sync for a long time, so maybe it's just not
> a big deal there.
>
> > My understanding, biased by my opinions, is that the preferred option
> > is to add a config variable controlling whether to use memcpy or a
> > packed struct. My preference, to avoid undefined behavior, is to
> > default to memcpy but set it to use packed structs for ARMv5. I will
> > vibe code this up, and apologize in advance for any offense such a
> > patch series may cause (ie I'm sure someone on LKML will want to
> > complain at me for using memcpy at all and give me a lecture about
> > strict aliasing being nonsense, etc.). Arnd or someone else may have a
> > different preferred solution, to which I'm more than happy to defer.
>
> As I wrote, we need the packed struct version at least on mips32,
> mips64, openrisc, riscv32, riscv64, sh4, sparc32, and sparc64 to avoid
> the external library calls the same as on armv5, but I would prefer
> to just always use that in the kernel. My guess is that this list
> is actually the same as the architectures that lack unaligned word
> access instructions in hardware, and armv5 is certainly not
> a special case.

Is it more than conjecture that these architectures have regressed? I
have a hard time believing the codegen on say riscv64 is lagging
aarch64. When the series originally landed the issue was looked at and
workarounds were added, for example, to PA-RISC.

> For 7.4 (and stable kernels) reverting the in-kernel header seems
> best to me, and then we can think about what to use for the tools.
>
> Looking at the uses of get_unaligned() in the tools directory,
> I notice that at least some of them should just not exist, e.g.
> struct debug_entry in tools/perf/util/jitdump.h is just
> needs a __packed attribute to correctly describe what it does,
> and tools/testing/cxl/test/mem.c does not need the unaligned
> helpers at all because the structs are all packed already.

There was supposed to be an 8-byte alignment requirement for perf
events and unfortunately some events broke this and we added __packed
as a workaround. The unaligned accesses wouldn't trip a sanitizer in
our tests because we sadly lack testing on old perf.data files. There
is a difference in using __packed as an attribute and using type
punning to achieve an unaligned access.

> I'm also a bit suspicious of the usage in
> tools/testing/selftests/bpf/test_kmods/bpf_testmod.c, which
> looks like it may run into a corner case:
>
> __bpf_kfunc int bpf_kfunc_test_uninit_alias(int *out__uninit, const int *in)
> {
>         int value = get_unaligned(in);
>         put_unaligned(42, out__uninit);
>         return value;
> }
>
> The current version of get_unaligned() turns into __builtin_memcpy(),
> and the function prototype defines both arguments as pointers
> to naturally aligned objects. Do you see anything that would
> stop the compiler from replacing the __builtin_memcpy() with
>  an invalid pointer dereference? This bit is probably fine
> because of the way gcc handles __builtin_memcpy() optimization,
> but I'm not sure if that's an implementation detail or required
> by the C standard, especially if strict aliasing rules say
> that the int* argument cannot overlap a misaligned int*.

I agree the pointers should be aligned given their types, ubsan
doesn't currently fail for this and waits for use of the pointer. So
there is the specification and also what sanitizers enforce.
For clang, when lowering from parsed C to machine code the compiler
will use alloca to model local variables and memcpy as an intrinsic
for aggregate copies. So:

  struct point { int x, y; } a, b;
  a = b; // Compiler will generate a builtin memcpy to copy b into a

This is why I have a hard time believing the lowering is broken on so
many architectures. Calling memcpy to copy aggregates everywhere would
be madness. I believe, especially given the period of time this code
has been in the kernel, that this problem is specific to GCC and
ARMv5. Clearly fixing a bug in GCC and ARMv5 should be done, but in
the meantime, a config option workaround in the kernel is pragmatic. I
also don't want to start a religious war, a solid -fno-strict-aliasing
stance, including with packed type punning quirks, I'm cool with. It
is better than libbpf that has a no -fno-strict-aliasing but let's
violate strict aliasing stance [1].

Thanks,
Ian

[1] https://lore.kernel.org/linux-perf-users/CAADnVQLKv6LOQw5=8_ZagRXdU7dRPYA+fxJsXiGncqJjZ2hK1w@xxxxxxxxxxxxxx/

>        Arnd