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

[Sam Su]

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