Re: [PATCH nf] netfilter: ip6t_rpfilter: handle routes without inet6_dev

Weiming Shi <[email protected]> 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))