Re: [PATCH bpf] libbpf: Fix undefined behavior in {get,put}_unaligned_be32()

[Yonghong Song <yonghong.song@linux.dev>]

On 10/5/25 6:20 PM, Eric Biggers wrote:
> These violate aliasing rules and may be miscompiled unless
> -fno-strict-aliasing is used.  Replace them with the standard memcpy()
> solution.  Note that compilers know how to optimize this properly.

For clang, -fstrict-aliasing is on by default. For gcc, -fstrict-aliasing
is on by default for >= O2. So indeed, with macro based implementation below,
it is *possible* that the compiler may explore strict-aliasing rule and may
generate incorrect code.

>
> Fixes: 4a1c9e544b8d ("libbpf: remove linux/unaligned.h dependency for libbpf_sha256()")
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>

Ack with a nit below.

Acked-by: Yonghong Song <yonghong.song@linux.dev>

> ---
>   tools/lib/bpf/libbpf_utils.c | 24 ++++++++++++++----------
>   1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf_utils.c b/tools/lib/bpf/libbpf_utils.c
> index 5d66bc6ff0982..ac3beae54cf67 100644
> --- a/tools/lib/bpf/libbpf_utils.c
> +++ b/tools/lib/bpf/libbpf_utils.c
> @@ -146,20 +146,24 @@ const char *libbpf_errstr(int err)
>   		snprintf(buf, sizeof(buf), "%d", err);
>   		return buf;
>   	}
>   }
>   
> -#pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Wpacked"
> -#pragma GCC diagnostic ignored "-Wattributes"
> -struct __packed_u32 { __u32 __val; } __attribute__((packed));
> -#pragma GCC diagnostic pop
> -
> -#define get_unaligned_be32(p) be32_to_cpu((((struct __packed_u32 *)(p))->__val))
> -#define put_unaligned_be32(v, p) do {							\
> -	((struct __packed_u32 *)(p))->__val = cpu_to_be32(v);				\
> -} while (0)
> +static inline __u32 get_unaligned_be32(const void *p)
> +{
> +	__be32 val;

To be consistent with put_unaligned_be32(), rename the above 'val'
to 'be_val'?

> +
> +	memcpy(&val, p, sizeof(val));
> +	return be32_to_cpu(val);
> +}
> +
> +static inline void put_unaligned_be32(__u32 val, void *p)
> +{
> +	__be32 be_val = cpu_to_be32(val);
> +
> +	memcpy(p, &be_val, sizeof(be_val));
> +}
>   
>   #define SHA256_BLOCK_LENGTH 64
>   #define Ch(x, y, z) (((x) & (y)) ^ (~(x) & (z)))
>   #define Maj(x, y, z) (((x) & (y)) ^ ((x) & (z)) ^ ((y) & (z)))
>   #define Sigma_0(x) (ror32((x), 2) ^ ror32((x), 13) ^ ror32((x), 22))
>
> base-commit: de7342228b7343774d6a9981c2ddbfb5e201044b