Re: [PATCH net] ipmr, ip6mr: queue netlink route queries out of band

[Paolo Abeni <pabeni@redhat.com>]

On 10/30/26 10:23, Eric Dumazet wrote:
> ipmr_get_route() and ip6mr_get_route() queue synthetic skbs on the
> unresolved MFC entry to answer RTM_GETROUTE requests once the route
> is resolved. These skbs carry a fake IP header with version 0, and
> ipmr_cache_resolve(), ipmr_destroy_unres(), ip6mr_cache_resolve()
> and ip6mr_destroy_unres() rely on this in-band tag to tell them
> apart from regular packets queued by the input and output paths.
> 
> The IP version is only validated once, in ip_rcv_core() and
> ip6_rcv_core(). A netfilter hook running later, for instance an
> nftables payload statement in a PREROUTING chain owned by an
> unprivileged user namespace, can rewrite the first header byte to
> zero. The SRv6 End.DX6 action also passes the decapsulated packet
> to ip6_mr_input() without checking that the inner header version
> is 6. If no MFC entry exists, the packet is queued and later handled
> as a netlink request: its payload is treated as an nlmsghdr and
> written to, mr_fill_mroute() may call skb_put() on a nonlinear skb
> (hitting SKB_LINEAR_ASSERT()), and the skb is unicast to a netlink
> portid taken from stale IPCB/IP6CB bytes in skb->cb[].
> 
> ipmr_cache_report() also relies on the same tag since
> commit 94a6736bb5ab ("ipmr: only copy pktinfo to cache reports").
> 
> Stop trusting packet contents: queue route queries on a dedicated
> nl_queries list in the unresolved MFC entry, and pass an explicit
> nl_query flag from the callers.
> 
> The per-entry limit of four queued skbs now applies to both lists
> combined, as before. The synthetic header keeps version 0, since it
> is copied into the NOCACHE report delivered to the mroute socket.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Fixes: 7bc570c8b4f7 ("[IPV6] MROUTE: Support multicast forwarding.")
> Reported-by: Katherine Leaver <kleaver@janestreet.com>
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> ---
>   include/linux/mroute_base.h |   2 +
>   net/ipv4/ipmr.c             | 104 +++++++++++++++++++-----------------
>   net/ipv6/ip6mr.c            |  76 +++++++++++++-------------
>   3 files changed, 94 insertions(+), 88 deletions(-)
> 
> diff --git a/include/linux/mroute_base.h b/include/linux/mroute_base.h
> index 4d55827e970537aff6938503a39ee3306c8fde6d..2d506966357b2591af4eadef88b27e236a2fe9cf 100644
> --- a/include/linux/mroute_base.h
> +++ b/include/linux/mroute_base.h
> @@ -119,6 +119,7 @@ enum {
>    * @mfc_flags: entry flags
>    * @expires: unresolved entry expire time
>    * @unresolved: unresolved cached skbs
> + * @nl_queries: queued RTM_GETROUTE requests waiting for resolution

FTR: the above causes kdoc warning, but fixing it would be so ugly:

   * @mfc_un.unres.nl_queries: ...

I really think we are better of this way.

/P