net: ipv4: fix ARM64 alignment fault in multipath hash seed

[PATCH v2] net: ipv4: fix ARM64 alignment fault in multipath hash seed

Posted by Yung Chih Su 1 week, 3 days ago

`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 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, thus executing it on a 4-byte
aligned address triggers a strict Alignment Fault (FSC = 0x21).

Fix the read side by moving the `READ_ONCE()` directly to the `u32`
member, which emits a safe 32-bit `ldar Wn`.

Furthermore, Eric Dumazet pointed out that `WRITE_ONCE()` on the entire
struct in `proc_fib_multipath_hash_set_seed()` is also flawed. Analysis
shows that Clang splits this 8-byte write into two separate 32-bit
`str` instructions. While this avoids an alignment fault, it destroys
atomicity and exposes a tear-write vulnerability. Fix this by
explicitly splitting the write into two 32-bit `WRITE_ONCE()`
operations.

Finally, add the missing `READ_ONCE()` when reading `user_seed` in
`proc_fib_multipath_hash_seed()` to ensure proper pairing and
concurrency safety.

Fixes: 4ee2a8cace3f ("net: ipv4: Add a sysctl to set multipath hash seed")
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Yung Chih Su <yuuchihsu@gmail.com>
---
v2:
  - Split WRITE_ONCE(struct) into two 32-bit WRITE_ONCE()s in 
    proc_fib_multipath_hash_set_seed() to fix a tear-write vulnerability.
  - Add missing READ_ONCE() for user_seed in proc_fib_multipath_hash_seed()
    per Eric Dumazet's suggestion.
  - Update Fixes tag to use the standard 12-char abbreviated format
    per Jakub Kicinski's suggestion.

 include/net/ip_fib.h       | 2 +-
 net/ipv4/sysctl_net_ipv4.c | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

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);
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 643763bc2142..5654cc9c8a0b 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -486,7 +486,8 @@ static void proc_fib_multipath_hash_set_seed(struct net *net, u32 user_seed)
 			    proc_fib_multipath_hash_rand_seed),
 	};
 
-	WRITE_ONCE(net->ipv4.sysctl_fib_multipath_hash_seed, new);
+	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);
 }
 
 static int proc_fib_multipath_hash_seed(const struct ctl_table *table, int write,
@@ -500,7 +501,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;
-- 
2.43.0

Re: [PATCH v2] net: ipv4: fix ARM64 alignment fault in multipath hash seed

Posted by Eric Dumazet 1 week, 3 days ago

On Mon, Mar 2, 2026 at 7:03 AM Yung Chih Su <yuuchihsu@gmail.com> 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 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, thus executing it on a 4-byte
> aligned address triggers a strict Alignment Fault (FSC = 0x21).
>
> Fix the read side by moving the `READ_ONCE()` directly to the `u32`
> member, which emits a safe 32-bit `ldar Wn`.
>
> Furthermore, Eric Dumazet pointed out that `WRITE_ONCE()` on the entire
> struct in `proc_fib_multipath_hash_set_seed()` is also flawed. Analysis
> shows that Clang splits this 8-byte write into two separate 32-bit
> `str` instructions. While this avoids an alignment fault, it destroys
> atomicity and exposes a tear-write vulnerability. Fix this by
> explicitly splitting the write into two 32-bit `WRITE_ONCE()`
> operations.
>
> Finally, add the missing `READ_ONCE()` when reading `user_seed` in
> `proc_fib_multipath_hash_seed()` to ensure proper pairing and
> concurrency safety.
>
> Fixes: 4ee2a8cace3f ("net: ipv4: Add a sysctl to set multipath hash seed")
> Suggested-by: Eric Dumazet <edumazet@google.com>

Nit: Use of Suggested-by: implies I made the original suggestion for
this patch (before the V1)
I simply gave a feedback on your V1. The idea of the patch came from you :)

So next time, do not add a "Suggested-by:", as the expected step is
that I add a Reviewed-by tag when I am happy with a new version,
if I see it in time before it is merged.

No need for a V3, this is simply a reminder for your next patches.

> Signed-off-by: Yung Chih Su <yuuchihsu@gmail.com>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks !

Re: [PATCH v2] net: ipv4: fix ARM64 alignment fault in multipath hash seed

Posted by Sam Su 1 week, 3 days ago

Hi Eric,

Got it! Thank you so much for the clarification and all your help with
the review.

Best regards,
Yung Chih.

Eric Dumazet <edumazet@google.com> 於 2026年3月2日週一 下午2:12寫道：

>
> On Mon, Mar 2, 2026 at 7:03 AM Yung Chih Su <yuuchihsu@gmail.com> 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 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, thus executing it on a 4-byte
> > aligned address triggers a strict Alignment Fault (FSC = 0x21).
> >
> > Fix the read side by moving the `READ_ONCE()` directly to the `u32`
> > member, which emits a safe 32-bit `ldar Wn`.
> >
> > Furthermore, Eric Dumazet pointed out that `WRITE_ONCE()` on the entire
> > struct in `proc_fib_multipath_hash_set_seed()` is also flawed. Analysis
> > shows that Clang splits this 8-byte write into two separate 32-bit
> > `str` instructions. While this avoids an alignment fault, it destroys
> > atomicity and exposes a tear-write vulnerability. Fix this by
> > explicitly splitting the write into two 32-bit `WRITE_ONCE()`
> > operations.
> >
> > Finally, add the missing `READ_ONCE()` when reading `user_seed` in
> > `proc_fib_multipath_hash_seed()` to ensure proper pairing and
> > concurrency safety.
> >
> > Fixes: 4ee2a8cace3f ("net: ipv4: Add a sysctl to set multipath hash seed")
> > Suggested-by: Eric Dumazet <edumazet@google.com>
>
> Nit: Use of Suggested-by: implies I made the original suggestion for
> this patch (before the V1)
> I simply gave a feedback on your V1. The idea of the patch came from you :)
>
> So next time, do not add a "Suggested-by:", as the expected step is
> that I add a Reviewed-by tag when I am happy with a new version,
> if I see it in time before it is merged.
>
> No need for a V3, this is simply a reminder for your next patches.
>
> > Signed-off-by: Yung Chih Su <yuuchihsu@gmail.com>
> > ---
>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
>
> Thanks !