ip_gre, ip6_gre: Fix race condition on o_seqno in collect_md mode
authorPeilin Ye <peilin.ye@bytedance.com>
Thu, 21 Apr 2022 22:09:02 +0000 (15:09 -0700)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Mon, 9 May 2022 07:14:36 +0000 (09:14 +0200)
commit6b59df7d4d0a132df7b549a165f01c586f50e4d8
treef7c0f3baafbfcda0b1ea9e9c0d45a72626c1cb01
parente5e985dd99d85ffa8a8758cd1db31e02421f009f
ip_gre, ip6_gre: Fix race condition on o_seqno in collect_md mode

[ Upstream commit 31c417c948d7f6909cb63f0ac3298f3c38f8ce20 ]

As pointed out by Jakub Kicinski, currently using TUNNEL_SEQ in
collect_md mode is racy for [IP6]GRE[TAP] devices.  Consider the
following sequence of events:

1. An [IP6]GRE[TAP] device is created in collect_md mode using "ip link
   add ... external".  "ip" ignores "[o]seq" if "external" is specified,
   so TUNNEL_SEQ is off, and the device is marked as NETIF_F_LLTX (i.e.
   it uses lockless TX);
2. Someone sets TUNNEL_SEQ on outgoing skb's, using e.g.
   bpf_skb_set_tunnel_key() in an eBPF program attached to this device;
3. gre_fb_xmit() or __gre6_xmit() processes these skb's:

gre_build_header(skb, tun_hlen,
 flags, protocol,
 tunnel_id_to_key32(tun_info->key.tun_id),
 (flags & TUNNEL_SEQ) ? htonl(tunnel->o_seqno++)
      : 0);   ^^^^^^^^^^^^^^^^^

Since we are not using the TX lock (&txq->_xmit_lock), multiple CPUs may
try to do this tunnel->o_seqno++ in parallel, which is racy.  Fix it by
making o_seqno atomic_t.

As mentioned by Eric Dumazet in commit b790e01aee74 ("ip_gre: lockless
xmit"), making o_seqno atomic_t increases "chance for packets being out
of order at receiver" when NETIF_F_LLTX is on.

Maybe a better fix would be:

1. Do not ignore "oseq" in external mode.  Users MUST specify "oseq" if
   they want the kernel to allow sequencing of outgoing packets;
2. Reject all outgoing TUNNEL_SEQ packets if the device was not created
   with "oseq".

Unfortunately, that would break userspace.

We could now make [IP6]GRE[TAP] devices always NETIF_F_LLTX, but let us
do it in separate patches to keep this fix minimal.

Suggested-by: Jakub Kicinski <kuba@kernel.org>
Fixes: 77a5196a804e ("gre: add sequence number for collect md mode.")
Signed-off-by: Peilin Ye <peilin.ye@bytedance.com>
Acked-by: William Tu <u9012063@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
include/net/ip6_tunnel.h
include/net/ip_tunnels.h
net/ipv4/ip_gre.c
net/ipv6/ip6_gre.c