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.
>
> 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>
