public inbox for linux-parisc@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/4] Switch get/put unaligned to use memcpy
@ 2025-10-16 20:51 Ian Rogers
  2025-10-16 20:51 ` [PATCH v5 1/4] parisc: Inline a type punning version of get_unaligned_le32 Ian Rogers
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Ian Rogers @ 2025-10-16 20:51 UTC (permalink / raw)
  To: James E.J. Bottomley, Helge Deller, Andy Lutomirski,
	Thomas Gleixner, Vincenzo Frascino, Ian Rogers,
	Arnaldo Carvalho de Melo, linux-parisc, linux-kernel,
	Eric Biggers, Al Viro, Christophe Leroy, Jason A. Donenfeld

The existing type punning approach with packed structs requires
 -fno-strict-aliasing to be passed to the compiler for
correctness. This is true in the kernel tree but was not true in the
tools directory until this patch from Eric Biggers <ebiggers@google.com>:
https://lore.kernel.org/lkml/20250625202311.23244-2-ebiggers@kernel.org/

Requiring -fno-strict-aliasing seems unfortunate and so this patch
makes the unaligned code work via memcpy rather than type punning with
the packed attribute.

v5: add a patch to make parisc still use a punned version of
    get_unaligned_le32 for an unusual boot case they have. This is
    untested but suggested as necessary by:
    https://lore.kernel.org/lkml/202509051042.7KOze0fZ-lkp@intel.com/
    I wasn't clear if this work was picked up, but I don't see it in
    v6.18-rc1 and so I'm resending rebased as v5.

v4: switch the type/expression variable __get_unaligned_ctrl_type that
    is used by _Generic to be a pointer to avoid 0 vs NULL usage
    warnings - always use NULL and dereference the type. This should
    also hopefully address analysis bots complaints.

v3: switch to __unqual_scalar_typeof, reducing the code, and use an
    uninitialized variable rather than a cast of 0 to try to avoid a
    sparse warning about not using NULL. The code is trying to
    navigate a minefield of uninitialized and casting warnings,
    hopefully the best balance has been struck, but the code will fail
    for cases like:
    const void *val = get_unaligned((const void * const *)ptr);
    due to __unqual_scalar_typeof leaving the 2nd const of the cast in
    place. Thankfully no code does this - tested with an
    allyesconfig. Support would be achievable by using void* as a
    default case in __unqual_scalar_typeof, it just doesn't seem worth
    it for a fairly unusual const case.

v2: switch memcpy to __builtin_memcpy to avoid potential/disallowed
    memcpy calls in vdso caused by -fno-builtin. Reported by
    Christophe Leroy <christophe.leroy@csgroup.eu>:
    https://lore.kernel.org/lkml/c57de5bf-d55c-48c5-9dfa-e2fb844dafe9@csgroup.eu/

Ian Rogers (4):
  parisc: Inline a type punning version of get_unaligned_le32
  vdso: Switch get/put unaligned from packed struct to memcpy
  tools headers: Update the linux/unaligned.h copy with the kernel
    sources
  tools headers: Remove unneeded ignoring of warnings in unaligned.h

 arch/parisc/boot/compressed/misc.c   | 15 +++++++++-
 include/vdso/unaligned.h             | 41 ++++++++++++++++++++++++----
 tools/include/linux/compiler_types.h | 22 +++++++++++++++
 tools/include/linux/unaligned.h      |  4 ---
 tools/include/vdso/unaligned.h       | 41 ++++++++++++++++++++++++----
 5 files changed, 106 insertions(+), 17 deletions(-)

-- 
2.51.0.858.gf9c4a03a3a-goog


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v5 1/4] parisc: Inline a type punning version of get_unaligned_le32
  2025-10-16 20:51 [PATCH v5 0/4] Switch get/put unaligned to use memcpy Ian Rogers
@ 2025-10-16 20:51 ` Ian Rogers
  2025-10-16 20:51 ` [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Ian Rogers
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Ian Rogers @ 2025-10-16 20:51 UTC (permalink / raw)
  To: James E.J. Bottomley, Helge Deller, Andy Lutomirski,
	Thomas Gleixner, Vincenzo Frascino, Ian Rogers,
	Arnaldo Carvalho de Melo, linux-parisc, linux-kernel,
	Eric Biggers, Al Viro, Christophe Leroy, Jason A. Donenfeld

Reading the byte/char output_len with get_unaligned_le32 can trigger
compiler warnings due to the size read. Avoid these warnings by using
type punning. This avoids issues when switching get_unaligned_t to
__builtin_memcpy.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 arch/parisc/boot/compressed/misc.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/parisc/boot/compressed/misc.c b/arch/parisc/boot/compressed/misc.c
index 9c83bd06ef15..111f267230a1 100644
--- a/arch/parisc/boot/compressed/misc.c
+++ b/arch/parisc/boot/compressed/misc.c
@@ -278,6 +278,19 @@ static void parse_elf(void *output)
 	free(phdrs);
 }
 
+/*
+ * The regular get_unaligned_le32 uses __builtin_memcpy which can trigger
+ * warnings when reading a byte/char output_len as an integer, as the size of a
+ * char is less than that of an integer. Use type punning and the packed
+ * attribute, which requires -fno-strict-aliasing, to work around the problem.
+ */
+static u32 punned_get_unaligned_le32(const void *p)
+{
+	const struct { __le32 x; } __packed * __get_pptr = p;
+
+	return le32_to_cpu(__get_pptr->x);
+}
+
 asmlinkage unsigned long __visible decompress_kernel(unsigned int started_wide,
 		unsigned int command_line,
 		const unsigned int rd_start,
@@ -309,7 +322,7 @@ asmlinkage unsigned long __visible decompress_kernel(unsigned int started_wide,
 	 * leave 2 MB for the stack.
 	 */
 	vmlinux_addr = (unsigned long) &_ebss + 2*1024*1024;
-	vmlinux_len = get_unaligned_le32(&output_len);
+	vmlinux_len = punned_get_unaligned_le32(&output_len);
 	output = (char *) vmlinux_addr;
 
 	/*
-- 
2.51.0.858.gf9c4a03a3a-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy
  2025-10-16 20:51 [PATCH v5 0/4] Switch get/put unaligned to use memcpy Ian Rogers
  2025-10-16 20:51 ` [PATCH v5 1/4] parisc: Inline a type punning version of get_unaligned_le32 Ian Rogers
@ 2025-10-16 20:51 ` Ian Rogers
  2025-10-19 17:24   ` David Laight
  2025-10-16 20:51 ` [PATCH v5 3/4] tools headers: Update the linux/unaligned.h copy with the kernel sources Ian Rogers
  2025-10-16 20:51 ` [PATCH v5 4/4] tools headers: Remove unneeded ignoring of warnings in unaligned.h Ian Rogers
  3 siblings, 1 reply; 6+ messages in thread
From: Ian Rogers @ 2025-10-16 20:51 UTC (permalink / raw)
  To: James E.J. Bottomley, Helge Deller, Andy Lutomirski,
	Thomas Gleixner, Vincenzo Frascino, Ian Rogers,
	Arnaldo Carvalho de Melo, linux-parisc, linux-kernel,
	Eric Biggers, Al Viro, Christophe Leroy, Jason A. Donenfeld

Type punning is necessary for get/put unaligned but the use of a
packed struct violates strict aliasing rules, requiring
-fno-strict-aliasing to be passed to the C compiler. Switch to using
memcpy so that -fno-strict-aliasing isn't necessary.

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

diff --git a/include/vdso/unaligned.h b/include/vdso/unaligned.h
index ff0c06b6513e..9076483c9fbb 100644
--- a/include/vdso/unaligned.h
+++ b/include/vdso/unaligned.h
@@ -2,14 +2,43 @@
 #ifndef __VDSO_UNALIGNED_H
 #define __VDSO_UNALIGNED_H
 
-#define __get_unaligned_t(type, ptr) ({							\
-	const struct { type x; } __packed * __get_pptr = (typeof(__get_pptr))(ptr);	\
-	__get_pptr->x;									\
+#include <linux/compiler_types.h>
+
+/**
+ * __get_unaligned_t - read an unaligned value from memory.
+ * @type:	the type to load from the pointer.
+ * @ptr:	the pointer to load from.
+ *
+ * Use memcpy to affect an unaligned type sized load avoiding undefined behavior
+ * from approaches like type punning that require -fno-strict-aliasing in order
+ * to be correct. As type may be const, use __unqual_scalar_typeof to map to a
+ * non-const type - you can't memcpy into a const type. The
+ * __get_unaligned_ctrl_type gives __unqual_scalar_typeof its required
+ * expression rather than type, a pointer is used to avoid warnings about mixing
+ * the use of 0 and NULL. The void* cast silences ubsan warnings.
+ */
+#define __get_unaligned_t(type, ptr) ({					\
+	type *__get_unaligned_ctrl_type __always_unused = NULL;		\
+	__unqual_scalar_typeof(*__get_unaligned_ctrl_type) __get_unaligned_val; \
+	__builtin_memcpy(&__get_unaligned_val, (void *)(ptr),		\
+			 sizeof(__get_unaligned_val));			\
+	__get_unaligned_val;						\
 })
 
-#define __put_unaligned_t(type, val, ptr) do {						\
-	struct { type x; } __packed * __put_pptr = (typeof(__put_pptr))(ptr);		\
-	__put_pptr->x = (val);								\
+/**
+ * __put_unaligned_t - write an unaligned value to memory.
+ * @type:	the type of the value to store.
+ * @val:	the value to store.
+ * @ptr:	the pointer to store to.
+ *
+ * Use memcpy to affect an unaligned type sized store avoiding undefined
+ * behavior from approaches like type punning that require -fno-strict-aliasing
+ * in order to be correct. The void* cast silences ubsan warnings.
+ */
+#define __put_unaligned_t(type, val, ptr) do {				\
+	type __put_unaligned_val = (val);				\
+	__builtin_memcpy((void *)(ptr), &__put_unaligned_val,		\
+			 sizeof(__put_unaligned_val));			\
 } while (0)
 
 #endif /* __VDSO_UNALIGNED_H */
-- 
2.51.0.858.gf9c4a03a3a-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v5 3/4] tools headers: Update the linux/unaligned.h copy with the kernel sources
  2025-10-16 20:51 [PATCH v5 0/4] Switch get/put unaligned to use memcpy Ian Rogers
  2025-10-16 20:51 ` [PATCH v5 1/4] parisc: Inline a type punning version of get_unaligned_le32 Ian Rogers
  2025-10-16 20:51 ` [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Ian Rogers
@ 2025-10-16 20:51 ` Ian Rogers
  2025-10-16 20:51 ` [PATCH v5 4/4] tools headers: Remove unneeded ignoring of warnings in unaligned.h Ian Rogers
  3 siblings, 0 replies; 6+ messages in thread
From: Ian Rogers @ 2025-10-16 20:51 UTC (permalink / raw)
  To: James E.J. Bottomley, Helge Deller, Andy Lutomirski,
	Thomas Gleixner, Vincenzo Frascino, Ian Rogers,
	Arnaldo Carvalho de Melo, linux-parisc, linux-kernel,
	Eric Biggers, Al Viro, Christophe Leroy, Jason A. Donenfeld

To pick up the changes in:

  vdso: Switch get/put unaligned from packed struct to memcpy

As the code is dependent on __unqual_scalar_typeof, update the tools
version of compiler_types.h to include this.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/include/linux/compiler_types.h | 22 +++++++++++++++
 tools/include/vdso/unaligned.h       | 41 ++++++++++++++++++++++++----
 2 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/tools/include/linux/compiler_types.h b/tools/include/linux/compiler_types.h
index d09f9dc172a4..890982283a5e 100644
--- a/tools/include/linux/compiler_types.h
+++ b/tools/include/linux/compiler_types.h
@@ -40,4 +40,26 @@
 #define asm_goto_output(x...) asm goto(x)
 #endif
 
+/*
+ * __unqual_scalar_typeof(x) - Declare an unqualified scalar type, leaving
+ *			       non-scalar types unchanged.
+ */
+/*
+ * Prefer C11 _Generic for better compile-times and simpler code. Note: 'char'
+ * is not type-compatible with 'signed char', and we define a separate case.
+ */
+#define __scalar_type_to_expr_cases(type)				\
+		unsigned type:	(unsigned type)0,			\
+		signed type:	(signed type)0
+
+#define __unqual_scalar_typeof(x) typeof(				\
+		_Generic((x),						\
+			 char:	(char)0,				\
+			 __scalar_type_to_expr_cases(char),		\
+			 __scalar_type_to_expr_cases(short),		\
+			 __scalar_type_to_expr_cases(int),		\
+			 __scalar_type_to_expr_cases(long),		\
+			 __scalar_type_to_expr_cases(long long),	\
+			 default: (x)))
+
 #endif /* __LINUX_COMPILER_TYPES_H */
diff --git a/tools/include/vdso/unaligned.h b/tools/include/vdso/unaligned.h
index ff0c06b6513e..9076483c9fbb 100644
--- a/tools/include/vdso/unaligned.h
+++ b/tools/include/vdso/unaligned.h
@@ -2,14 +2,43 @@
 #ifndef __VDSO_UNALIGNED_H
 #define __VDSO_UNALIGNED_H
 
-#define __get_unaligned_t(type, ptr) ({							\
-	const struct { type x; } __packed * __get_pptr = (typeof(__get_pptr))(ptr);	\
-	__get_pptr->x;									\
+#include <linux/compiler_types.h>
+
+/**
+ * __get_unaligned_t - read an unaligned value from memory.
+ * @type:	the type to load from the pointer.
+ * @ptr:	the pointer to load from.
+ *
+ * Use memcpy to affect an unaligned type sized load avoiding undefined behavior
+ * from approaches like type punning that require -fno-strict-aliasing in order
+ * to be correct. As type may be const, use __unqual_scalar_typeof to map to a
+ * non-const type - you can't memcpy into a const type. The
+ * __get_unaligned_ctrl_type gives __unqual_scalar_typeof its required
+ * expression rather than type, a pointer is used to avoid warnings about mixing
+ * the use of 0 and NULL. The void* cast silences ubsan warnings.
+ */
+#define __get_unaligned_t(type, ptr) ({					\
+	type *__get_unaligned_ctrl_type __always_unused = NULL;		\
+	__unqual_scalar_typeof(*__get_unaligned_ctrl_type) __get_unaligned_val; \
+	__builtin_memcpy(&__get_unaligned_val, (void *)(ptr),		\
+			 sizeof(__get_unaligned_val));			\
+	__get_unaligned_val;						\
 })
 
-#define __put_unaligned_t(type, val, ptr) do {						\
-	struct { type x; } __packed * __put_pptr = (typeof(__put_pptr))(ptr);		\
-	__put_pptr->x = (val);								\
+/**
+ * __put_unaligned_t - write an unaligned value to memory.
+ * @type:	the type of the value to store.
+ * @val:	the value to store.
+ * @ptr:	the pointer to store to.
+ *
+ * Use memcpy to affect an unaligned type sized store avoiding undefined
+ * behavior from approaches like type punning that require -fno-strict-aliasing
+ * in order to be correct. The void* cast silences ubsan warnings.
+ */
+#define __put_unaligned_t(type, val, ptr) do {				\
+	type __put_unaligned_val = (val);				\
+	__builtin_memcpy((void *)(ptr), &__put_unaligned_val,		\
+			 sizeof(__put_unaligned_val));			\
 } while (0)
 
 #endif /* __VDSO_UNALIGNED_H */
-- 
2.51.0.858.gf9c4a03a3a-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v5 4/4] tools headers: Remove unneeded ignoring of warnings in unaligned.h
  2025-10-16 20:51 [PATCH v5 0/4] Switch get/put unaligned to use memcpy Ian Rogers
                   ` (2 preceding siblings ...)
  2025-10-16 20:51 ` [PATCH v5 3/4] tools headers: Update the linux/unaligned.h copy with the kernel sources Ian Rogers
@ 2025-10-16 20:51 ` Ian Rogers
  3 siblings, 0 replies; 6+ messages in thread
From: Ian Rogers @ 2025-10-16 20:51 UTC (permalink / raw)
  To: James E.J. Bottomley, Helge Deller, Andy Lutomirski,
	Thomas Gleixner, Vincenzo Frascino, Ian Rogers,
	Arnaldo Carvalho de Melo, linux-parisc, linux-kernel,
	Eric Biggers, Al Viro, Christophe Leroy, Jason A. Donenfeld

Now the get/put unaligned use memcpy the -Wpacked and -Wattributes
warnings don't need disabling.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/include/linux/unaligned.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tools/include/linux/unaligned.h b/tools/include/linux/unaligned.h
index 395a4464fe73..d51ddafed138 100644
--- a/tools/include/linux/unaligned.h
+++ b/tools/include/linux/unaligned.h
@@ -6,9 +6,6 @@
  * This is the most generic implementation of unaligned accesses
  * and should work almost anywhere.
  */
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wpacked"
-#pragma GCC diagnostic ignored "-Wattributes"
 #include <vdso/unaligned.h>
 
 #define get_unaligned(ptr)	__get_unaligned_t(typeof(*(ptr)), (ptr))
@@ -143,6 +140,5 @@ static inline u64 get_unaligned_be48(const void *p)
 {
 	return __get_unaligned_be48(p);
 }
-#pragma GCC diagnostic pop
 
 #endif /* __LINUX_UNALIGNED_H */
-- 
2.51.0.858.gf9c4a03a3a-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy
  2025-10-16 20:51 ` [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Ian Rogers
@ 2025-10-19 17:24   ` David Laight
  0 siblings, 0 replies; 6+ messages in thread
From: David Laight @ 2025-10-19 17:24 UTC (permalink / raw)
  To: Ian Rogers
  Cc: James E.J. Bottomley, Helge Deller, Andy Lutomirski,
	Thomas Gleixner, Vincenzo Frascino, Arnaldo Carvalho de Melo,
	linux-parisc, linux-kernel, Eric Biggers, Al Viro,
	Christophe Leroy, Jason A. Donenfeld

On Thu, 16 Oct 2025 13:51:24 -0700
Ian Rogers <irogers@google.com> wrote:

> Type punning is necessary for get/put unaligned but the use of a
> packed struct violates strict aliasing rules, requiring
> -fno-strict-aliasing to be passed to the C compiler. Switch to using
> memcpy so that -fno-strict-aliasing isn't necessary.

Does the compiler always manage to optimise everything away?
You really do need it to generate the code for a misaligned
memory access.

You might be better off removing the 'strict-aliasing' warning
by 'laundering' the pointer through an integer type (probably long).

	David

> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  include/vdso/unaligned.h | 41 ++++++++++++++++++++++++++++++++++------
>  1 file changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/include/vdso/unaligned.h b/include/vdso/unaligned.h
> index ff0c06b6513e..9076483c9fbb 100644
> --- a/include/vdso/unaligned.h
> +++ b/include/vdso/unaligned.h
> @@ -2,14 +2,43 @@
>  #ifndef __VDSO_UNALIGNED_H
>  #define __VDSO_UNALIGNED_H
>  
> -#define __get_unaligned_t(type, ptr) ({							\
> -	const struct { type x; } __packed * __get_pptr = (typeof(__get_pptr))(ptr);	\
> -	__get_pptr->x;									\
> +#include <linux/compiler_types.h>
> +
> +/**
> + * __get_unaligned_t - read an unaligned value from memory.
> + * @type:	the type to load from the pointer.
> + * @ptr:	the pointer to load from.
> + *
> + * Use memcpy to affect an unaligned type sized load avoiding undefined behavior
> + * from approaches like type punning that require -fno-strict-aliasing in order
> + * to be correct. As type may be const, use __unqual_scalar_typeof to map to a
> + * non-const type - you can't memcpy into a const type. The
> + * __get_unaligned_ctrl_type gives __unqual_scalar_typeof its required
> + * expression rather than type, a pointer is used to avoid warnings about mixing
> + * the use of 0 and NULL. The void* cast silences ubsan warnings.
> + */
> +#define __get_unaligned_t(type, ptr) ({					\
> +	type *__get_unaligned_ctrl_type __always_unused = NULL;		\
> +	__unqual_scalar_typeof(*__get_unaligned_ctrl_type) __get_unaligned_val; \
> +	__builtin_memcpy(&__get_unaligned_val, (void *)(ptr),		\
> +			 sizeof(__get_unaligned_val));			\
> +	__get_unaligned_val;						\
>  })
>  
> -#define __put_unaligned_t(type, val, ptr) do {						\
> -	struct { type x; } __packed * __put_pptr = (typeof(__put_pptr))(ptr);		\
> -	__put_pptr->x = (val);								\
> +/**
> + * __put_unaligned_t - write an unaligned value to memory.
> + * @type:	the type of the value to store.
> + * @val:	the value to store.
> + * @ptr:	the pointer to store to.
> + *
> + * Use memcpy to affect an unaligned type sized store avoiding undefined
> + * behavior from approaches like type punning that require -fno-strict-aliasing
> + * in order to be correct. The void* cast silences ubsan warnings.
> + */
> +#define __put_unaligned_t(type, val, ptr) do {				\
> +	type __put_unaligned_val = (val);				\
> +	__builtin_memcpy((void *)(ptr), &__put_unaligned_val,		\
> +			 sizeof(__put_unaligned_val));			\
>  } while (0)
>  
>  #endif /* __VDSO_UNALIGNED_H */


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-10-19 17:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-16 20:51 [PATCH v5 0/4] Switch get/put unaligned to use memcpy Ian Rogers
2025-10-16 20:51 ` [PATCH v5 1/4] parisc: Inline a type punning version of get_unaligned_le32 Ian Rogers
2025-10-16 20:51 ` [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Ian Rogers
2025-10-19 17:24   ` David Laight
2025-10-16 20:51 ` [PATCH v5 3/4] tools headers: Update the linux/unaligned.h copy with the kernel sources Ian Rogers
2025-10-16 20:51 ` [PATCH v5 4/4] tools headers: Remove unneeded ignoring of warnings in unaligned.h Ian Rogers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox