mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH nf] netfilter: ip6t_rpfilter: handle routes without inet6_dev
@ 2026-09-06  3:49 Weiming Shi
  2026-09-06  4:18 ` Florian Westphal
  2026-09-09 18:52 ` netdev-bot+sashiko
  0 siblings, 2 replies; 4+ messages in thread
From: Weiming Shi @ 2026-09-06  3:49 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Florian Westphal, Phil Sutter,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: netfilter-devel, coreteam, netdev, linux-kernel,
	co+459f67f4d8af8ce6, Xiang Mei

ip6_route_lookup() can return an error-free route whose rt6i_idev is
NULL.  For example, lowering an external nexthop device's MTU below
IPV6_MIN_MTU tears down its inet6_dev while fib6_ifdown() leaves routes
using nexthop objects in the FIB.

rpfilter_lookup_reverse6() dereferences rt6i_idev before evaluating its
loose-mode condition.  This lets an unprivileged user with a private user
and network namespace trigger a NULL pointer dereference:

  Oops: general protection fault, probably for non-canonical address
  0xdffffc0000000000
  KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
  RIP: rpfilter_mt
  ip6t_do_table
  nf_hook_slow
  ipv6_rcv
  process_backlog
  net_rx_action
  handle_softirqs

Evaluate loose mode first because route existence is sufficient there.
In strict mode, compare devices only when the route has an inet6_dev.

Fixes: e26f9a480fb6 ("netfilter: add ipv6 reverse path filter match")
Reported-by: co+459f67f4d8af8ce6@bugs.sh
Closes: https://lore.kernel.org/all/VtWUkE8QzJt5CroTj2V2v3ZQ0gwbXZ7nq7I3@bugs.sh/
Assisted-by: Claude:gpt-5
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 net/ipv6/netfilter/ip6t_rpfilter.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/netfilter/ip6t_rpfilter.c b/net/ipv6/netfilter/ip6t_rpfilter.c
index 67c87a88cde4f..2bcc96f9385cb 100644
--- a/net/ipv6/netfilter/ip6t_rpfilter.c
+++ b/net/ipv6/netfilter/ip6t_rpfilter.c
@@ -72,9 +72,11 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
 		goto out;
 	}
 
-	if (rt->rt6i_idev->dev == dev ||
-	    l3mdev_master_ifindex_rcu(rt->rt6i_idev->dev) == dev->ifindex ||
-	    (flags & XT_RPFILTER_LOOSE))
+	if (flags & XT_RPFILTER_LOOSE)
+		ret = true;
+	else if (rt->rt6i_idev &&
+		 (rt->rt6i_idev->dev == dev ||
+		  l3mdev_master_ifindex_rcu(rt->rt6i_idev->dev) == dev->ifindex))
 		ret = true;
  out:
 	ip6_rt_put(rt);
-- 
2.55.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH nf] netfilter: ip6t_rpfilter: handle routes without inet6_dev
  2026-09-06  3:49 [PATCH nf] netfilter: ip6t_rpfilter: handle routes without inet6_dev Weiming Shi
@ 2026-09-06  4:18 ` Florian Westphal
  2026-09-06  7:46   ` Weiming Shi
  2026-09-09 18:52 ` netdev-bot+sashiko
  1 sibling, 1 reply; 4+ messages in thread
From: Florian Westphal @ 2026-09-06  4:18 UTC (permalink / raw)
  To: Weiming Shi
  Cc: Pablo Neira Ayuso, Phil Sutter, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, netfilter-devel,
	coreteam, netdev, linux-kernel, co+459f67f4d8af8ce6, Xiang Mei

Weiming Shi <bestswngs@gmail.com> wrote:
> ip6_route_lookup() can return an error-free route whose rt6i_idev is
> NULL.  For example, lowering an external nexthop device's MTU below
> IPV6_MIN_MTU tears down its inet6_dev while fib6_ifdown() leaves routes
> using nexthop objects in the FIB.
> 
> rpfilter_lookup_reverse6() dereferences rt6i_idev before evaluating its
> loose-mode condition.  This lets an unprivileged user with a private user
> and network namespace trigger a NULL pointer dereference:
> 
>   Oops: general protection fault, probably for non-canonical address
>   0xdffffc0000000000
>   KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
>   RIP: rpfilter_mt
>   ip6t_do_table
>   nf_hook_slow
>   ipv6_rcv
>   process_backlog
>   net_rx_action
>   handle_softirqs
> 
> Evaluate loose mode first because route existence is sufficient there.
> In strict mode, compare devices only when the route has an inet6_dev.

I don't think we should treat rt->rt6i_idev == NULL as an eligible
result, even in loose mode. Maybe this instead?

diff --git a/net/ipv6/netfilter/ip6t_rpfilter.c b/net/ipv6/netfilter/ip6t_rpfilter.c
--- a/net/ipv6/netfilter/ip6t_rpfilter.c
+++ b/net/ipv6/netfilter/ip6t_rpfilter.c
@@ -61,7 +61,7 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
                fl6.flowi6_oif = dev->ifindex;
 
        rt = (void *)ip6_route_lookup(net, &fl6, skb, lookup_flags);
-       if (rt->dst.error)
+       if (rt->dst.error || !rt->rt6i_idev)
                goto out;
 
        if (rt->rt6i_flags & (RTF_REJECT|RTF_ANYCAST))



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH nf] netfilter: ip6t_rpfilter: handle routes without inet6_dev
  2026-09-06  4:18 ` Florian Westphal
@ 2026-09-06  7:46   ` Weiming Shi
  0 siblings, 0 replies; 4+ messages in thread
From: Weiming Shi @ 2026-09-06  7:46 UTC (permalink / raw)
  To: Florian Westphal
  Cc: Pablo Neira Ayuso, Phil Sutter, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, netfilter-devel,
	coreteam, netdev, linux-kernel, co+459f67f4d8af8ce6, Xiang Mei

Florian Westphal <fw@strlen.de> 于2026年9月6日周日 12:18写道:
>
> Weiming Shi <bestswngs@gmail.com> wrote:
> > ip6_route_lookup() can return an error-free route whose rt6i_idev is
> > NULL.  For example, lowering an external nexthop device's MTU below
> > IPV6_MIN_MTU tears down its inet6_dev while fib6_ifdown() leaves routes
> > using nexthop objects in the FIB.
> >
> > rpfilter_lookup_reverse6() dereferences rt6i_idev before evaluating its
> > loose-mode condition.  This lets an unprivileged user with a private user
> > and network namespace trigger a NULL pointer dereference:
> >
> >   Oops: general protection fault, probably for non-canonical address
> >   0xdffffc0000000000
> >   KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> >   RIP: rpfilter_mt
> >   ip6t_do_table
> >   nf_hook_slow
> >   ipv6_rcv
> >   process_backlog
> >   net_rx_action
> >   handle_softirqs
> >
> > Evaluate loose mode first because route existence is sufficient there.
> > In strict mode, compare devices only when the route has an inet6_dev.
>
> I don't think we should treat rt->rt6i_idev == NULL as an eligible
> result, even in loose mode. Maybe this instead?
>
> diff --git a/net/ipv6/netfilter/ip6t_rpfilter.c b/net/ipv6/netfilter/ip6t_rpfilter.c
> --- a/net/ipv6/netfilter/ip6t_rpfilter.c
> +++ b/net/ipv6/netfilter/ip6t_rpfilter.c
> @@ -61,7 +61,7 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
>                 fl6.flowi6_oif = dev->ifindex;
>
>         rt = (void *)ip6_route_lookup(net, &fl6, skb, lookup_flags);
> -       if (rt->dst.error)
> +       if (rt->dst.error || !rt->rt6i_idev)
>                 goto out;
>
>         if (rt->rt6i_flags & (RTF_REJECT|RTF_ANYCAST))
>
>

 Thanks, agreed. v2 will be sent.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH nf] netfilter: ip6t_rpfilter: handle routes without inet6_dev
  2026-09-06  3:49 [PATCH nf] netfilter: ip6t_rpfilter: handle routes without inet6_dev Weiming Shi
  2026-09-06  4:18 ` Florian Westphal
@ 2026-09-09 18:52 ` netdev-bot+sashiko
  1 sibling, 0 replies; 4+ messages in thread
From: netdev-bot+sashiko @ 2026-09-09 18:52 UTC (permalink / raw)
  To: bestswngs
  Cc: pablo, fw, phil, davem, edumazet, kuba, pabeni, horms,
	netfilter-devel, coreteam, netdev, linux-kernel,
	co+459f67f4d8af8ce6, xmei5

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] net/ipv6/netfilter/ip6t_rpfilter.c:rpfilter_lookup_reverse6()
  now makes strict-mode matching depend on the presence of an optional…

--

netfilter: ip6t_rpfilter: handle routes without inet6_dev

This reorders rpfilter_lookup_reverse6() so loose mode is evaluated before
touching rt->rt6i_idev, and adds a NULL check on rt6i_idev before the strict
mode device comparison.  The stated trigger is a route using an external
nexthop object whose device lost its inet6_dev.

Closes: https://lore.kernel.org/all/VtWUkE8QzJt5CroTj2V2v3ZQ0gwbXZ7nq7I3@bugs.sh/

> diff --git a/net/ipv6/netfilter/ip6t_rpfilter.c b/net/ipv6/netfilter/ip6t_rpfilter.c
> index 67c87a88cde4f..2bcc96f9385cb 100644
> --- a/net/ipv6/netfilter/ip6t_rpfilter.c
> +++ b/net/ipv6/netfilter/ip6t_rpfilter.c
> @@ -72,9 +72,11 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
>  		goto out;
>  	}
>  
> -	if (rt->rt6i_idev->dev == dev ||
> -	    l3mdev_master_ifindex_rcu(rt->rt6i_idev->dev) == dev->ifindex ||
> -	    (flags & XT_RPFILTER_LOOSE))
> +	if (flags & XT_RPFILTER_LOOSE)
> +		ret = true;
> +	else if (rt->rt6i_idev &&
> +		 (rt->rt6i_idev->dev == dev ||
> +		  l3mdev_master_ifindex_rcu(rt->rt6i_idev->dev) == dev->ifindex))
>  		ret = true;

[Severity: Medium]
Should the strict-mode comparison use the route's nexthop device instead of
rt6i_idev?  With this form, strict mode returns no-match for exactly the
routes the commit message describes, i.e. those whose device has no
inet6_dev, even when the packet did arrive on that device.

rt6i_idev is filled in best-effort in ip6_rt_copy_init():

net/ipv6/route.c:ip6_rt_copy_init() {
	...
	rt->rt6i_idev = dev ? in6_dev_get(dev) : NULL;
	...
}

while the device itself is always present on the returned route, since
ip6_create_rt_rcu() allocates it from res->nh->fib_nh_dev:

net/ipv6/route.c:ip6_create_rt_rcu() {
	struct net_device *dev = res->nh->fib_nh_dev;
	...
	nrt = ip6_dst_alloc(dev_net(dev), dev, flags);
	...
}

Would comparing rt->dst.dev keep the strict-mode contract and make the NULL
guard unnecessary?

Does this also make the two in-tree implementations of the same reverse path
semantics disagree?  nft_fib_ipv6.c derives the device from the fib6
nexthops and never looks at rt6i_idev:

net/ipv6/netfilter/nft_fib_ipv6.c:nft_fib6_info_nh_dev_match() {
	return nh_dev == dev ||
	       l3mdev_master_ifindex_rcu(nh_dev) == dev->ifindex;
}

and the IPv4 half of the same match does the same thing:

net/ipv4/netfilter/ipt_rpfilter.c:rpfilter_lookup_reverse() {
	...
	return fib_info_nh_uses_dev(res.fi, dev) || flags & XT_RPFILTER_LOOSE;
}

So for one topology, nft fib saddr . iif oif would still match while
iptables -m rpfilter in strict mode would not, and the common
-m rpfilter --invert -j DROP rule would then drop the packet.

Two paths where the verdict looks like it changes rather than staying
incidentally the same:

1) The VRF second pass.  vrf_ip6_rcv() re-runs PRE_ROUTING with dev set to
the VRF master:

drivers/net/vrf.c:vrf_ip6_rcv() {
	...
	skb = vrf_rcv_nfhook(NFPROTO_IPV6, NF_INET_PRE_ROUTING, skb, vrf_dev);
	...
}

If the reverse route's nexthop is an enslaved device whose inet6_dev was
torn down, and the fib6_info uses an external nexthop object so the route
survives:

net/ipv6/route.c:fib6_ifdown() {
	...
	if (rt == net->ipv6.fib6_null_entry || rt->nh)
		return 0;
	...
}

then the l3mdev_master_ifindex_rcu() clause that used to accept this is now
skipped entirely.

2) A cached exception route (RTF_CACHE, e.g. a PMTU exception) created while
the nexthop device had no inet6_dev keeps rt6i_idev NULL even after the
device's inet6_dev is recreated, so a packet arriving on that very device is
reported as not matching.

Is the NULL rt6i_idev case meant to be treated as "route exists but device
unknown", or is the intent that the check simply never needed rt6i_idev in
the first place?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260906034939.3746540-1-bestswngs%40gmail.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-09 18:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-06  3:49 [PATCH nf] netfilter: ip6t_rpfilter: handle routes without inet6_dev Weiming Shi
2026-09-06  4:18 ` Florian Westphal
2026-09-06  7:46   ` Weiming Shi
2026-09-09 18:52 ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®