Re: [PATCH] net: ipv6: fix ARM64 alignment fault in fib_multipath_hash_from_keys()

[Eric Dumazet]

On Thu, Feb 26, 2026 at 3:50 PM Sam Su <yuuchihsu@xxxxxxxxx> wrote:
>
> Eric Dumazet <edumazet@xxxxxxxxxx> 於 2026年2月26日週四 下午8:02寫道：
> >
> > On Thu, Feb 26, 2026 at 12:18 PM Yung Chih Su <yuuchihsu@xxxxxxxxx> wrote:
> > >
> > > struct sysctl_fib_multipath_hash_seed contains two u32 fields (user_seed
> > > and mp_seed), making it an 8-byte structure with a 4-byte alignment requirement.
> > >
> > > In fib_multipath_hash_from_keys(), the code evaluates the entire struct
> > > atomically via READ_ONCE():
> > >
> > > mp_seed = READ_ONCE(net->ipv4.sysctl_fib_multipath_hash_seed).mp_seed;
> > > While this silently works on GCC by falling back to unaligned regular loads
> > > (e.g., LDR/LDUR) which the ARM64 kernel tolerates, it causes a fatal kernel
> > > panic when compiled with Clang and LTO enabled.
> > >
> > > Commit e35123d83ee3 ("arm64: lto: Strengthen READ_ONCE() to acquire when
> > > CONFIG_LTO=y") strengthens READ_ONCE() to use Load-Acquire instructions
> > > (ldar / ldapr) to prevent compiler reordering bugs under Clang LTO.
> > >
> > > Since the macro evaluates the full 8-byte struct, Clang emits a 64-bit
> > > ldar instruction. ARM64 architecture strictly requires ldar to be
> > > naturally aligned. Executing a 64-bit ldar on a 4-byte aligned address
> > > (e.g., ending in 0xEC) triggers a strict Alignment Fault (FSC = 0x21).
> > >
> > > Fix this by moving the READ_ONCE() directly to the specific u32 member:
> > >
> > > mp_seed = READ_ONCE(net->ipv4.sysctl_fib_multipath_hash_seed.mp_seed);
> > > This instructs the compiler to emit a 32-bit load (ldar Wn or ldr Wn),
> > > which perfectly satisfies the 4-byte alignment requirement and resolves
> > > the crash.
> > >
> > > Fixes: [4ee2a8cace3fb9a34aea6a56426f89d26dd514f3] ("net: ipv4: Add a sysctl to set multipath hash seed")
> > > Signed-off-by: Yung Chih Su <yuuchihsu@xxxxxxxxx>
> > > ---
> > >  include/net/ip_fib.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> > > index b4495c38e0a0..318593743b6e 100644
> > > --- a/include/net/ip_fib.h
> > > +++ b/include/net/ip_fib.h
> > > @@ -559,7 +559,7 @@ static inline u32 fib_multipath_hash_from_keys(const struct net *net,
> > >         siphash_aligned_key_t hash_key;
> > >         u32 mp_seed;
> > >
> > > -       mp_seed = READ_ONCE(net->ipv4.sysctl_fib_multipath_hash_seed).mp_seed;
> > > +       mp_seed = READ_ONCE(net->ipv4.sysctl_fib_multipath_hash_seed.mp_seed);
> > >         fib_multipath_hash_construct_key(&hash_key, mp_seed);
> > >
> > >         return flow_hash_from_keys_seed(keys, &hash_key);
> >
> > What about proc_fib_multipath_hash_set_seed() ?
> >
> > It has :
> >
> > WRITE_ONCE(net->ipv4.sysctl_fib_multipath_hash_seed, new);
> >
> > Which is IMO strange, regardless of ARM64 clang and whats not.
>
> Hi Eric,
>
> Thank you for taking a look and catching this!
>
> You are absolutely right. If READ_ONCE() on this 4-byte aligned struct
> causes an unaligned load-acquire (ldar) fault on ARM64, the
> WRITE_ONCE() in proc_fib_multipath_hash_set_seed() will inevitably
> cause an unaligned store-release (stlr) fault when a user tries to
> modify the sysctl. Using WRITE_ONCE() on an entire struct here is
> indeed structurally flawed and unsafe.
>
> To fix the write side properly, I should write the members
> individually to ensure safe 32-bit atomic operations. I am thinking of
> updating proc_fib_multipath_hash_set_seed() to something like this:
>
>     WRITE_ONCE(net->ipv4.sysctl_fib_multipath_hash_seed.user_seed,
> new.user_seed);

SGTM, but please add this part as well:
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 643763bc214279047c90b5b92a9ba9be6c24a443..1974b826bd9451fd9d8054e1db811760ff4b5a9f
100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -500,7 +500,7 @@ static int proc_fib_multipath_hash_seed(const
struct ctl_table *table, int write
        int ret;

        mphs = &net->ipv4.sysctl_fib_multipath_hash_seed;
-       user_seed = mphs->user_seed;
+       user_seed = READ_ONCE(mphs->user_seed);

        tmp = *table;
        tmp.data = &user_seed;

>     WRITE_ONCE(net->ipv4.sysctl_fib_multipath_hash_seed.mp_seed, new.mp_seed);
>
> I would like to take a little bit of time to reproduce the
> WRITE_ONCE() crash on my ARM64 device and thoroughly test this
> proposed fix to ensure everything works correctly.
> Once I confirm the fix is solid and runs perfectly on the hardware, I
> will submit the v2 patch addressing both the read and write sides of
> this unaligned struct issue.
> Thanks again for the insightful review!
>
> Best regards,
> Yung Chih.

Thanks !