Skip to content

Commit fc8b2a6

Browse files
wdebruijdavem330
authored andcommitted
net: more strict VIRTIO_NET_HDR_GSO_UDP_L4 validation
Syzbot reported two new paths to hit an internal WARNING using the new virtio gso type VIRTIO_NET_HDR_GSO_UDP_L4. RIP: 0010:skb_checksum_help+0x4a2/0x600 net/core/dev.c:3260 skb len=64521 gso_size=344 and RIP: 0010:skb_warn_bad_offload+0x118/0x240 net/core/dev.c:3262 Older virtio types have historically had loose restrictions, leading to many entirely impractical fuzzer generated packets causing problems deep in the kernel stack. Ideally, we would have had strict validation for all types from the start. New virtio types can have tighter validation. Limit UDP GSO packets inserted via virtio to the same limits imposed by the UDP_SEGMENT socket interface: 1. must use checksum offload 2. checksum offload matches UDP header 3. no more segments than UDP_MAX_SEGMENTS 4. UDP GSO does not take modifier flags, notably SKB_GSO_TCP_ECN Fixes: 860b7f2 ("linux/virtio_net.h: Support USO offload in vnet header.") Reported-by: [email protected] Closes: https://lore.kernel.org/netdev/[email protected]/ Reported-by: [email protected] Closes: https://lore.kernel.org/netdev/[email protected]/ Signed-off-by: Willem de Bruijn <[email protected]> Reviewed-by: Eric Dumazet <[email protected]> Acked-by: Jason Wang <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 2f3389c commit fc8b2a6

File tree

1 file changed

+16
-3
lines changed

1 file changed

+16
-3
lines changed

include/linux/virtio_net.h

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
#define _LINUX_VIRTIO_NET_H
44

55
#include <linux/if_vlan.h>
6+
#include <linux/udp.h>
67
#include <uapi/linux/tcp.h>
7-
#include <uapi/linux/udp.h>
88
#include <uapi/linux/virtio_net.h>
99

1010
static inline bool virtio_net_hdr_match_proto(__be16 protocol, __u8 gso_type)
@@ -151,9 +151,22 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
151151
unsigned int nh_off = p_off;
152152
struct skb_shared_info *shinfo = skb_shinfo(skb);
153153

154-
/* UFO may not include transport header in gso_size. */
155-
if (gso_type & SKB_GSO_UDP)
154+
switch (gso_type & ~SKB_GSO_TCP_ECN) {
155+
case SKB_GSO_UDP:
156+
/* UFO may not include transport header in gso_size. */
156157
nh_off -= thlen;
158+
break;
159+
case SKB_GSO_UDP_L4:
160+
if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM))
161+
return -EINVAL;
162+
if (skb->csum_offset != offsetof(struct udphdr, check))
163+
return -EINVAL;
164+
if (skb->len - p_off > gso_size * UDP_MAX_SEGMENTS)
165+
return -EINVAL;
166+
if (gso_type != SKB_GSO_UDP_L4)
167+
return -EINVAL;
168+
break;
169+
}
157170

158171
/* Kernel has a special handling for GSO_BY_FRAGS. */
159172
if (gso_size == GSO_BY_FRAGS)

0 commit comments

Comments
 (0)