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

[David Laight]

On Mon, 19 Oct 2026 18:27:24 +0200
"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.
> 
> 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.
> 
> 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?

Historically it has definitely done that.
And you can't 'cast away' the alignment implicit in a type.
The alignment can get tracked into an inlined function.
Plenty of crashes on sparc32 from that one.

Any structure that might be misaligned in memory better be packed.

But what is actually wrong with using byte access? eg:
u32 get_le32(u8 *p) { return p[0] | p[1] << 8 | p[2] << 16 | p[3] << 24; }
On x86, arm64, ppc (etc) both gcc and clang generate a single memory read
(byteswapped if necessary).

https://godbolt.org/z/d33hvesr8

David


> 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*.
> 
>        Arnd
>