virtio_net_l3_start() decides from skb->dev->type whether the frame
starts with an Ethernet header. tun decides it from the IFF_TAP and
IFF_TUN flags, and TUNSETLINK can set the type of a down tun device
to any value, without changing the flags.
On an IFF_TAP device set to ARPHRD_NONE, the outer protocol checks
look at the IP header at the network header (ETH_HLEN), and never read
the Ethernet header. eth_type_trans() then sets skb->protocol from an
unchecked h_proto, for instance ETH_P_NSH, and the packet can reach
the skb_under_panic() addressed by the previous patch.
On an IFF_TUN device set to ARPHRD_ETHER, the checks validate an
Ethernet header that tun then treats as the start of the L3 header.
Checking in tun that the device type matches the framing is not
enough: the type is read again later, and a concurrent TUNSETLINK can
change it in between, since tun_get_user() only drops packets of a
down device after these checks.
Add a has_eth argument to virtio_net_hdr_tnl_to_skb(), passed down to
virtio_net_l3_start(), so the device type is no longer used:
- virtio_net always receives Ethernet frames.
- tun_get_user() passes whether the device is IFF_TAP.
- tun_xdp_one() always calls eth_type_trans() on its frames.
Fixes: 288f30435132 ("tun: enable gso over UDP tunnel support.")
Reported-by: Sashiko <[email protected]>
Closes: https://lore.kernel.org/netdev/[email protected]/
Link:
https://lore.kernel.org/netdev/[email protected]/
Signed-off-by: Eric Dumazet <[email protected]>
---
drivers/net/tun.c | 9 +++++++--
drivers/net/tun_vnet.h | 5 +++--
drivers/net/virtio_net.c | 2 +-
include/linux/virtio_net.h | 27 +++++++++++++--------------
4 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index dcec7311e601..3920b3dff1e8 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1944,7 +1944,8 @@ static ssize_t tun_get_user(struct tun_struct *tun,
struct tun_file *tfile,
break;
}
- if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
+ if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr,
+ (tun->flags & TUN_TYPE_MASK) == IFF_TAP)) {
atomic_long_inc(&tun->rx_frame_errors);
err = -EINVAL;
goto free_skb;
@@ -2621,7 +2622,11 @@ static int tun_xdp_one(struct tun_struct *tun,
tnl_hdr = (struct virtio_net_hdr_v1_hash_tunnel *)gso;
skb->dev = tun->dev;
skb_set_network_header(skb, ETH_HLEN);
- if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr)) {
+ /* Frames always start with an Ethernet header here, as the
+ * eth_type_trans() call below expects.
+ */
+ if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr,
+ true)) {
atomic_long_inc(&tun->rx_frame_errors);
kfree_skb(skb);
ret = -EINVAL;
diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h
index f4c652b1fa44..3364e08dd6e0 100644
--- a/drivers/net/tun_vnet.h
+++ b/drivers/net/tun_vnet.h
@@ -197,9 +197,10 @@ static inline netdev_features_t
tun_vnet_hdr_guest_features(int vnet_hdr_sz)
static inline int
tun_vnet_hdr_tnl_to_skb(unsigned int flags, netdev_features_t features,
struct sk_buff *skb,
- const struct virtio_net_hdr_v1_hash_tunnel *hdr)
+ const struct virtio_net_hdr_v1_hash_tunnel *hdr,
+ bool has_eth)
{
- return virtio_net_hdr_tnl_to_skb(skb, hdr,
+ return virtio_net_hdr_tnl_to_skb(skb, hdr, has_eth,
features & NETIF_F_GSO_UDP_TUNNEL,
features & NETIF_F_GSO_UDP_TUNNEL_CSUM,
tun_vnet_is_little_endian(flags));
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index dd2c41ef6fbe..ed20e76fbc4c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2533,7 +2533,7 @@ static void virtnet_receive_done(struct virtnet_info *vi,
struct receive_queue *
skb->dev = dev;
skb_set_network_header(skb, ETH_HLEN);
- if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl,
+ if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, true, vi->rx_tnl,
vi->rx_tnl_csum,
virtio_is_little_endian(vi->vdev))) {
net_warn_ratelimited("%s: bad gso: type: %x, size: %u, flags %x
tunnel %d tnl csum %d\n",
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 357116c2a148..b7b6b542f476 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -87,20 +87,18 @@ static inline struct iphdr *virtio_net_ip4_parse(struct
sk_buff *skb,
}
/* Where does the L3 header start, and what protocol is it?
- * Handles Ethernet and VLAN headers; other devices use the
- * network header offset the caller set up.
+ * With @has_eth, the frame starts with an Ethernet header, possibly
+ * followed by VLAN tags. Otherwise the L3 header starts at the network
+ * header offset the caller set up.
+ *
+ * The caller knows how the frame will be parsed later: the device
+ * type does not tell, as tun can change it with TUNSETLINK.
*/
static inline bool
-virtio_net_l3_start(struct sk_buff *skb, int *nhoff, __be16 *proto)
+virtio_net_l3_start(struct sk_buff *skb, bool has_eth, int *nhoff,
+ __be16 *proto)
{
- /* skb->dev should always be set here already,
- * but let's not crash when it is not.
- */
- if (WARN_ON_ONCE(!skb->dev))
- return false;
-
- if (skb->dev->type == ARPHRD_ETHER ||
- skb->dev->type == ARPHRD_LOOPBACK) {
+ if (has_eth) {
__be16 h_proto;
if (!pskb_may_pull(skb, ETH_HLEN))
@@ -431,7 +429,7 @@ static inline unsigned int virtio_l3min(bool is_ipv6)
* carries, with UDP following it at @outer_th.
*/
static inline bool
-virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool outer_isv6,
+virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool has_eth, bool outer_isv6,
unsigned int outer_th)
{
int nhoff;
@@ -439,7 +437,7 @@ virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool
outer_isv6,
unsigned int thoff;
int ip_proto;
- if (!virtio_net_l3_start(skb, &nhoff, &proto))
+ if (!virtio_net_l3_start(skb, has_eth, &nhoff, &proto))
return false;
if (outer_isv6 != (proto == htons(ETH_P_IPV6)))
return false;
@@ -452,6 +450,7 @@ virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool
outer_isv6,
static inline int
virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
const struct virtio_net_hdr_v1_hash_tunnel *vhdr,
+ bool has_eth,
bool tnl_hdr_negotiated,
bool tnl_csum_negotiated,
bool little_endian)
@@ -505,7 +504,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
if (ret)
return ret;
- if (!virtio_net_tnl_offsets_ok(skb, outer_isv6, outer_th))
+ if (!virtio_net_tnl_offsets_ok(skb, has_eth, outer_isv6, outer_th))
return -EINVAL;
/* In case of USO, the inner protocol is still unknown and
--
2.53.0
