* [PATCH bpf] libbpf: Fix undefined behavior in {get,put}_unaligned_be32()
@ 2025-10-06 1:20 Eric Biggers
2025-10-06 16:12 ` Yonghong Song
0 siblings, 1 reply; 3+ messages in thread
From: Eric Biggers @ 2025-10-06 1:20 UTC (permalink / raw)
To: bpf, Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau
Cc: Song Liu, Yonghong Song, John Fastabend, KP Singh,
Stanislav Fomichev, Hao Luo, Jiri Olsa, Tony Ambardar,
Eric Biggers
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.
Fixes: 4a1c9e544b8d ("libbpf: remove linux/unaligned.h dependency for libbpf_sha256()")
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
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;
+
+ 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
--
2.51.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH bpf] libbpf: Fix undefined behavior in {get,put}_unaligned_be32()
2025-10-06 1:20 [PATCH bpf] libbpf: Fix undefined behavior in {get,put}_unaligned_be32() Eric Biggers
@ 2025-10-06 16:12 ` Yonghong Song
2025-10-06 16:18 ` Andrii Nakryiko
0 siblings, 1 reply; 3+ messages in thread
From: Yonghong Song @ 2025-10-06 16:12 UTC (permalink / raw)
To: Eric Biggers, bpf, Andrii Nakryiko, Eduard Zingerman,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau
Cc: Song Liu, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo,
Jiri Olsa, Tony Ambardar
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH bpf] libbpf: Fix undefined behavior in {get,put}_unaligned_be32()
2025-10-06 16:12 ` Yonghong Song
@ 2025-10-06 16:18 ` Andrii Nakryiko
0 siblings, 0 replies; 3+ messages in thread
From: Andrii Nakryiko @ 2025-10-06 16:18 UTC (permalink / raw)
To: Yonghong Song
Cc: Eric Biggers, bpf, Andrii Nakryiko, Eduard Zingerman,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Tony Ambardar
On Mon, Oct 6, 2025 at 9:13 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
>
> 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>
>
we raced a bit, I just applied this patch a few minutes before your
ack. I added your ack and force-pushed
> > ---
> > 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'?
I left this as is, it's fine, IMO.
>
> > +
> > + 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
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-10-06 16:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-06 1:20 [PATCH bpf] libbpf: Fix undefined behavior in {get,put}_unaligned_be32() Eric Biggers
2025-10-06 16:12 ` Yonghong Song
2025-10-06 16:18 ` Andrii Nakryiko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.