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

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

[Ian Rogers]

Commit a339671db64b ("vdso: Switch get/put_unaligned() from packed struct
to memcpy()") switched __get_unaligned_t() and __put_unaligned_t() from
using a __packed struct to __builtin_memcpy() to avoid strict aliasing
undefined behavior.

On targets without efficient unaligned memory accesses (such as ARMv5),
GCC may not inline __builtin_memcpy() into byte shifts and instead emit
stack spills or much slower out-of-line memcpy() calls. Add
CONFIG_VDSO_UNALIGNED_PACKED, defaulting to
!HAVE_EFFICIENT_UNALIGNED_ACCESS, to use the __packed struct
implementation on strict-alignment targets while keeping
__builtin_memcpy() elsewhere and in tools/.

Fixes: a339671db64b ("vdso: Switch get/put_unaligned() from packed struct to memcpy()")
Reported-by: Stefan Kerkmann <s.kerkmann@pengutronix.de>
Closes: https://lore.kernel.org/lkml/47f8c30b-e326-4ff0-a2bc-5683209a9e2d@pengutronix.de/
Signed-off-by: Ian Rogers <irogers@google.com>
---
v2: Default VDSO_UNALIGNED_PACKED to !HAVE_EFFICIENT_UNALIGNED_ACCESS
    instead of CPU_32v5 to cover all strict-alignment targets (Sashiko,
    Arnd Bergmann).

 include/vdso/unaligned.h | 16 ++++++++++++++++
 lib/vdso/Kconfig         | 10 ++++++++++
 2 files changed, 26 insertions(+)

diff --git a/include/vdso/unaligned.h b/include/vdso/unaligned.h
index 9076483c9fbb..726fd15efc82 100644
--- a/include/vdso/unaligned.h
+++ b/include/vdso/unaligned.h
@@ -4,6 +4,20 @@
 
 #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)
+
+#else /* !CONFIG_VDSO_UNALIGNED_PACKED */
+
 /**
  * __get_unaligned_t - read an unaligned value from memory.
  * @type:	the type to load from the pointer.
@@ -41,4 +55,6 @@
 			 sizeof(__put_unaligned_val));			\
 } while (0)
 
+#endif /* CONFIG_VDSO_UNALIGNED_PACKED */
+
 #endif /* __VDSO_UNALIGNED_H */
diff --git a/lib/vdso/Kconfig b/lib/vdso/Kconfig
index 597f5f0f9681..2436c20ee0ef 100644
--- a/lib/vdso/Kconfig
+++ b/lib/vdso/Kconfig
@@ -23,3 +23,13 @@ config VDSO_GETRANDOM
 	select VDSO_DATASTORE
 	help
 	  Selected by architectures that support vDSO getrandom().
+
+config VDSO_UNALIGNED_PACKED
+	bool
+	default !HAVE_EFFICIENT_UNALIGNED_ACCESS
+	help
+	  Use packed structs rather than memcpy() in include/vdso/unaligned.h
+	  for unaligned memory accesses. Enabled by default when the
+	  architecture does not have efficient unaligned accesses, as GCC may
+	  emit stack spills or out-of-line memcpy() calls instead of inlining
+	  byte shifts.
-- 
2.56.0.212.g129ede20d9-goog

[PATCH v2 2/2] tools headers: Update the vdso/unaligned.h copy with the kernel sources

[Ian Rogers]

To pick up the changes in:

  vdso: Add VDSO_UNALIGNED_PACKED config option

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/include/vdso/unaligned.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/include/vdso/unaligned.h b/tools/include/vdso/unaligned.h
index 9076483c9fbb..726fd15efc82 100644
--- a/tools/include/vdso/unaligned.h
+++ b/tools/include/vdso/unaligned.h
@@ -4,6 +4,20 @@
 
 #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)
+
+#else /* !CONFIG_VDSO_UNALIGNED_PACKED */
+
 /**
  * __get_unaligned_t - read an unaligned value from memory.
  * @type:	the type to load from the pointer.
@@ -41,4 +55,6 @@
 			 sizeof(__put_unaligned_val));			\
 } while (0)
 
+#endif /* CONFIG_VDSO_UNALIGNED_PACKED */
+
 #endif /* __VDSO_UNALIGNED_H */
-- 
2.56.0.212.g129ede20d9-goog

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

[David Laight]

On Tue, 20 Oct 2026 09:32:09 -0700
Ian Rogers <irogers@google.com> wrote:

> Commit a339671db64b ("vdso: Switch get/put_unaligned() from packed struct
> to memcpy()") switched __get_unaligned_t() and __put_unaligned_t() from
> using a __packed struct to __builtin_memcpy() to avoid strict aliasing
> undefined behavior.
> 
> On targets without efficient unaligned memory accesses (such as ARMv5),
> GCC may not inline __builtin_memcpy() into byte shifts and instead emit
> stack spills or much slower out-of-line memcpy() calls. Add
> CONFIG_VDSO_UNALIGNED_PACKED, defaulting to
> !HAVE_EFFICIENT_UNALIGNED_ACCESS, to use the __packed struct
> implementation on strict-alignment targets while keeping
> __builtin_memcpy() elsewhere and in tools/.
> 
> Fixes: a339671db64b ("vdso: Switch get/put_unaligned() from packed struct to memcpy()")
> Reported-by: Stefan Kerkmann <s.kerkmann@pengutronix.de>
> Closes: https://lore.kernel.org/lkml/47f8c30b-e326-4ff0-a2bc-5683209a9e2d@pengutronix.de/
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> v2: Default VDSO_UNALIGNED_PACKED to !HAVE_EFFICIENT_UNALIGNED_ACCESS
>     instead of CPU_32v5 to cover all strict-alignment targets (Sashiko,
>     Arnd Bergmann).
> 
>  include/vdso/unaligned.h | 16 ++++++++++++++++
>  lib/vdso/Kconfig         | 10 ++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/include/vdso/unaligned.h b/include/vdso/unaligned.h
> index 9076483c9fbb..726fd15efc82 100644
> --- a/include/vdso/unaligned.h
> +++ b/include/vdso/unaligned.h
> @@ -4,6 +4,20 @@
>  
>  #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).

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.

If you write the value as u64 then read as u32 I think they could cross
even though the buffer is u8[].
That might mean you need a barrier() before the read in get() and before
and after the write in put().
(Partial barriers may work - dunno.)

David


> +
> +#else /* !CONFIG_VDSO_UNALIGNED_PACKED */
> +
>  /**
>   * __get_unaligned_t - read an unaligned value from memory.
>   * @type:	the type to load from the pointer.
> @@ -41,4 +55,6 @@
>  			 sizeof(__put_unaligned_val));			\
>  } while (0)
>  
> +#endif /* CONFIG_VDSO_UNALIGNED_PACKED */
> +
>  #endif /* __VDSO_UNALIGNED_H */
> diff --git a/lib/vdso/Kconfig b/lib/vdso/Kconfig
> index 597f5f0f9681..2436c20ee0ef 100644
> --- a/lib/vdso/Kconfig
> +++ b/lib/vdso/Kconfig
> @@ -23,3 +23,13 @@ config VDSO_GETRANDOM
>  	select VDSO_DATASTORE
>  	help
>  	  Selected by architectures that support vDSO getrandom().
> +
> +config VDSO_UNALIGNED_PACKED
> +	bool
> +	default !HAVE_EFFICIENT_UNALIGNED_ACCESS
> +	help
> +	  Use packed structs rather than memcpy() in include/vdso/unaligned.h
> +	  for unaligned memory accesses. Enabled by default when the
> +	  architecture does not have efficient unaligned accesses, as GCC may
> +	  emit stack spills or out-of-line memcpy() calls instead of inlining
> +	  byte shifts.

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

[Arnd Bergmann]

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.

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.

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

     Arnd

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

[David Laight]

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

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

[Arnd Bergmann]

On Sat, Oct 24, 2026, at 10:32, David Laight wrote:
> 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:  
>>
>> 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.

I have definitely seen the reverse, where a u32 pointer dereference in
__raw_readl() gets turned into individual byte accesses because
the pointer originally came from a packed structure. This is
also bad because it breaks MMIO register access.

We also know that the packed struct version works, either because
the standard requires a punned pointer to respect the minimum
alignment, or because the implementation is smart enough to only
ever lower the alignment for punned pointers.

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

Right, especially the memcpy() without type punning seems dangerous.
We have had several instances of code breaking on armv7 where
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set but the stp/ldp
instructions for 64-bit objects still require alignment.

I don't see any instances of the exact issue you describe, but this
one is similar and definitely broken on armv7 (adding the dw_mmc
maintainers to Cc for information):

#ifndef readq
#define __raw_writeq(__value, __reg) \
        (*(volatile u64 __force *)(__reg) = (__value))
#define __raw_readq(__reg) (*(volatile u64 __force *)(__reg))
#endif
#define mci_fifo_writeq(__value, __reg) __raw_writeq(__reg, __value)
static void dw_mci_push_data64(struct dw_mci *host, void *buf, int cnt)
{
...
#ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
        if (unlikely((unsigned long)buf & 0x7)) {
                while (cnt >= 8) {
                        u64 aligned_buf[16];
                        int len = min(cnt & -8, (int)sizeof(aligned_buf));
                        int items = len >> 3;
                        int i;
                        /* memcpy from input buffer into aligned buffer */
                        memcpy(aligned_buf, buf, len);
                        buf += len;
                        cnt -= len;
                        /* push data from aligned buffer into fifo */
                        for (i = 0; i < items; ++i)
                                mci_fifo_writeq(host->fifo_reg, aligned_buf[i]);
                }
        } else
#endif
        {
                u64 *pdata = buf;

                for (; cnt >= 8; cnt -= 8)
                        mci_fifo_writeq(host->fifo_reg, *pdata++);
                buf = pdata;
        }

The 'u64 *pdata = buf' cast is of course broken here since that
is not allowed for unaligned pointers on supported target regardless
of CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. The else{...} portion
has to be written using get_unaligned_u64() here, as the #ifndef
branch should only optimize for performance and not be required
for correctness.
If '*buf' was a u64 pointer instead of void, I could see the
compiler messing this up in the way you describe, by optimizing
out the temporary array entirely.

An unrelated problem is that the mci_fifo_writeq() definition
is not portable because the volatile u64 pointer dereference
does not guarantee that the two 32-bit halves are stored
in the order that the hardware register requires. Presumably
that was what led to DW_MMC_QUIRK_FIFO64_32.

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

Right, we do rely on that in e.g. x86

typedef s64 __attribute__((aligned(4))) compat_s64;

We cannot use this in a macro though because there is no
way to define a local typedef inside of a function or
to change the alignment of pointer without the struct.

       Arnd