* [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®