Eric Dumazet wrote:
> On Fri, Jun 7, 2024 at 10:36 AM Florian Westphal <fw@xxxxxxxxx> wrote:
> >
> > Years ago flow dissector gained ability to delegate flow dissection
> > to a bpf program, scoped per netns.
> >
> > Unfortunately, skb_get_hash() only gets an sk_buff argument instead
> > of both net+skb. This means the flow dissector needs to obtain the
> > netns pointer from somewhere else.
> >
> > The netns is derived from skb->dev, and if that is not available, from
> > skb->sk. If neither is set, we hit a (benign) WARN_ON_ONCE().
> >
> > Trying both dev and sk covers most cases, but not all, as recently
> > reported by Christoph Paasch.
> >
> > In case of nf-generated tcp reset, both sk and dev are NULL:
> >
> > WARNING: .. net/core/flow_dissector.c:1104
> > skb_flow_dissect_flow_keys include/linux/skbuff.h:1536 [inline]
> > skb_get_hash include/linux/skbuff.h:1578 [inline]
> > nft_trace_init+0x7d/0x120 net/netfilter/nf_tables_trace.c:320
> > nft_do_chain+0xb26/0xb90 net/netfilter/nf_tables_core.c:268
> > nft_do_chain_ipv4+0x7a/0xa0 net/netfilter/nft_chain_filter.c:23
> > nf_hook_slow+0x57/0x160 net/netfilter/core.c:626
> > __ip_local_out+0x21d/0x260 net/ipv4/ip_output.c:118
> > ip_local_out+0x26/0x1e0 net/ipv4/ip_output.c:127
> > nf_send_reset+0x58c/0x700 net/ipv4/netfilter/nf_reject_ipv4.c:308
> > nft_reject_ipv4_eval+0x53/0x90 net/ipv4/netfilter/nft_reject_ipv4.c:30
> > [..]
> >
> > syzkaller did something like this:
> > table inet filter {
> > chain input {
> > type filter hook input priority filter; policy accept;
> > meta nftrace set 1 # calls skb_get_hash
> > tcp dport 42 reject with tcp reset # emits skb with NULL skb dev/sk
> > }
> > chain output {
> > type filter hook output priority filter; policy accept;
> > # empty chain is enough
> > }
> > }
> >
> > ... then sends a tcp packet to port 42.
> >
> > Initial attempt to simply set skb->dev from nf_reject_ipv4 doesn't cover
> > all cases: skbs generated via ipv4 igmp_send_report trigger similar splat.
Does this mean we have more non-nf callsites to convert?
> >
> > Moreover, Pablo Neira found that nft_hash.c uses __skb_get_hash_symmetric()
> > which would trigger same warn splat for such skbs.
> >
> > Lets allow callers to pass the current netns explicitly.
> > The nf_trace infrastructure is adjusted to use the new helper.
> >
> > __skb_get_hash_symmetric is handled in the next patch.
> >
> > Reported-by: Christoph Paasch <cpaasch@xxxxxxxxx>
> > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/494
> > Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
>
> Nice, I had an internal syzbot report about the same issue.
>
> Reviewed-by: Eric Dumazet <edumazet@xxxxxxxxxx>
Subject to the documentation warning from the bot
Reviewed-by: Willem de Bruijn <willemb@xxxxxxxxxx>
Thanks for fixing this, Florian.
