Re: [PATCH v2 1/2] vdso: Add VDSO_UNALIGNED_PACKED config option

[David Laight <david.laight.linux@gmail.com>]

On Fri, 23 Oct 2026 23:49:50 +0200
"Arnd Bergmann" <arnd@arndb.de> wrote:

> On Tue, Oct 20, 2026, at 20:07, David Laight wrote:
> > On Tue, 20 Oct 2026 09:32:09 -0700 Ian Rogers <irogers@google.com> wrote:  
> 
> >>  
> >>  #include <linux/compiler_types.h>
> >>  
> >> +#ifdef CONFIG_VDSO_UNALIGNED_PACKED
> >> +
> >> +#define __get_unaligned_t(type, ptr) ({							\
> >> +	const struct { type x; } __packed * __get_pptr = (typeof(__get_pptr))(ptr);	\
> >> +	__get_pptr->x;									\
> >> +})
> >> +
> >> +#define __put_unaligned_t(type, val, ptr) do {						\
> >> +	struct { type x; } __packed * __put_pptr = (typeof(__put_pptr))(ptr);		\
> >> +	__put_pptr->x = (val);								\
> >> +} while (0)  
> >
> > I'm confused (easily done).
> > That code relies on strict alignment being disabled regardless of whether
> > misaligned accesses fault or not.
> >
> > So it is either always valid or always wrong.
> >
> > Additionally I think the compiler can sometimes track the alignment through
> > casts (although it didn't for a quick check I did earlier).  
> 
> This is the traditional way we have been doing the unaligned access
> in the kernel before the change. By annotating the structure as
> __packed, we tell the compiler that it cannot rely on alignment here,
> so it uses whatever method it knows to access unaligned data depending
> on the architecture and flags.

That relies on the compiler not tracking the alignment through the casts.
The only way the C language lets you get a 'foo *' pointer is taking the
address of a 'foo' data item so it must be aligned.
So even if you cast it to another type that alignment can be assumed.
I've seen crashes from memcpy() using word accesses when trying to
copy from a misaligned pointer into an aligned copy of the structure.
Now the current code must be avoiding that - but I think the compiler
can do it.

> The reason we need the struct here is that you cannot use __packed
> as a qualifier on a variable, only on a type or struct member.

I know all about that.
Somewhere it also says that you can only increase the alignment (I think
of a type), but you can definitely have a type for 'u64 with 32bit
alignment' and use it in a structure.

> 
> > Since having misaligned non-packed structures really doesn't work, the only
> > places these functions should be used is on u8[] buffers.
> > Pointers into those can (probably) be cast to packed structure types
> > without causing issues if strict aliasing is enabled.  
> 
> strict aliasing is thankfully never enabled in the kernel.
> 
> The cast is from a higher alignment to byte alignment, and that
> is always allowed unlike the reverse. Technically the only
> reason we ever need the get_unaligned() helpers is to work
> around pointers to objects that should exist because the
> ABI requires them to be aligned but the pointer instead
> came out of some byte stream or from taking a (non-__packed)
> pointer of a __packed struct member.

I think those ought to be found and marked packed.
I doubt any of them get used other than part of the byte stream.
That leaves the explicit get_unaligned_le32() used to pull 32bit
values from a byte stream (that isn't defined by a structure).

But we're probably arguing in basic agreement.  

> 
>      Arnd