[PATCH net] ipv4: nexthop: update has_v4 flag for any family change in replace_nexthop_single()

[PATCH net] ipv4: nexthop: update has_v4 flag for any family change in replace_nexthop_single()

[Xiang Mei]

When a nexthop within a group is replaced, nh_group_v4_update() is only
called when the old nexthop is AF_INET and the new one is AF_INET6. The
reverse direction (AF_INET6 to AF_INET) is not handled, leaving the
group's has_v4 flag stale at false.

This causes fib6_check_nexthop() to incorrectly accept an IPv6 route
referencing a group that now contains an AF_INET nexthop. During route
lookup, nexthop_fib6_nh() returns NULL for the AF_INET nexthop and the
subsequent dereference in rt6_find_cached_rt() crashes with a general
protection fault:

 Oops: general protection fault, probably for non-canonical address [...]
 KASAN: null-ptr-deref in range [0x0000000000000050-0x0000000000000057]
 RIP: 0010:ip6_pol_route
 Call Trace:
  fib6_rule_lookup
  ip6_route_output_flags
  inet6_rtm_getroute
  rtnetlink_rcv_msg
  [...]

Fix by calling nh_group_v4_update() whenever the old and new nexthops
have different address families, not just for AF_INET to AF_INET6.
Using a general inequality is safe here: individual nexthops can only
be AF_INET or AF_INET6 (enforced at parse time), so the only family
transitions possible are between these two, and nh_group_v4_update()
is a full rescan that always produces the correct has_v4 value.

Fixes: 885a3b15791d ("ipv4: nexthop: Correctly update nexthop group when replacing a nexthop")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
 net/ipv4/nexthop.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index 2c9036c719b6..b2ea15446cd2 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -2466,10 +2466,10 @@ static int replace_nexthop_single(struct net *net, struct nexthop *old,
 			goto err_notify;
 	}
 
-	/* When replacing an IPv4 nexthop with an IPv6 nexthop, potentially
-	 * update IPv4 indication in all the groups using the nexthop.
+	/* When the nexthop family changes, update the IPv4 indication in all
+	 * the groups using the nexthop.
 	 */
-	if (oldi->family == AF_INET && newi->family == AF_INET6) {
+	if (oldi->family != newi->family) {
 		list_for_each_entry(nhge, &old->grp_list, nh_list) {
 			struct nexthop *nhp = nhge->nh_parent;
 			struct nh_group *nhg;
-- 
2.43.0

Re: [PATCH net] ipv4: nexthop: update has_v4 flag for any family change in replace_nexthop_single()

[David Ahern]

On 4/8/26 12:28 PM, Xiang Mei wrote:
> When a nexthop within a group is replaced, nh_group_v4_update() is only
> called when the old nexthop is AF_INET and the new one is AF_INET6. The
> reverse direction (AF_INET6 to AF_INET) is not handled, leaving the
> group's has_v4 flag stale at false.
> 
> This causes fib6_check_nexthop() to incorrectly accept an IPv6 route
> referencing a group that now contains an AF_INET nexthop. During route
> lookup, nexthop_fib6_nh() returns NULL for the AF_INET nexthop and the
> subsequent dereference in rt6_find_cached_rt() crashes with a general
> protection fault:
> 

...

> 
> Fix by calling nh_group_v4_update() whenever the old and new nexthops
> have different address families, not just for AF_INET to AF_INET6.
> Using a general inequality is safe here: individual nexthops can only
> be AF_INET or AF_INET6 (enforced at parse time), so the only family
> transitions possible are between these two, and nh_group_v4_update()
> is a full rescan that always produces the correct has_v4 value.
> 

please add a test to tools/testing/selftests/net/fib_nexthops.sh
covering this use case.