Skip to content

Commit 89add40

Browse files
wdebruijkuba-moo
authored andcommitted
net: drop bad gso csum_start and offset in virtio_net_hdr
Tighten csum_start and csum_offset checks in virtio_net_hdr_to_skb for GSO packets. The function already checks that a checksum requested with VIRTIO_NET_HDR_F_NEEDS_CSUM is in skb linear. But for GSO packets this might not hold for segs after segmentation. Syzkaller demonstrated to reach this warning in skb_checksum_help offset = skb_checksum_start_offset(skb); ret = -EINVAL; if (WARN_ON_ONCE(offset >= skb_headlen(skb))) By injecting a TSO packet: WARNING: CPU: 1 PID: 3539 at net/core/dev.c:3284 skb_checksum_help+0x3d0/0x5b0 ip_do_fragment+0x209/0x1b20 net/ipv4/ip_output.c:774 ip_finish_output_gso net/ipv4/ip_output.c:279 [inline] __ip_finish_output+0x2bd/0x4b0 net/ipv4/ip_output.c:301 iptunnel_xmit+0x50c/0x930 net/ipv4/ip_tunnel_core.c:82 ip_tunnel_xmit+0x2296/0x2c70 net/ipv4/ip_tunnel.c:813 __gre_xmit net/ipv4/ip_gre.c:469 [inline] ipgre_xmit+0x759/0xa60 net/ipv4/ip_gre.c:661 __netdev_start_xmit include/linux/netdevice.h:4850 [inline] netdev_start_xmit include/linux/netdevice.h:4864 [inline] xmit_one net/core/dev.c:3595 [inline] dev_hard_start_xmit+0x261/0x8c0 net/core/dev.c:3611 __dev_queue_xmit+0x1b97/0x3c90 net/core/dev.c:4261 packet_snd net/packet/af_packet.c:3073 [inline] The geometry of the bad input packet at tcp_gso_segment: [ 52.003050][ T8403] skb len=12202 headroom=244 headlen=12093 tailroom=0 [ 52.003050][ T8403] mac=(168,24) mac_len=24 net=(192,52) trans=244 [ 52.003050][ T8403] shinfo(txflags=0 nr_frags=1 gso(size=1552 type=3 segs=0)) [ 52.003050][ T8403] csum(0x60000c7 start=199 offset=1536 ip_summed=3 complete_sw=0 valid=0 level=0) Mitigate with stricter input validation. csum_offset: for GSO packets, deduce the correct value from gso_type. This is already done for USO. Extend it to TSO. Let UFO be: udp[46]_ufo_fragment ignores these fields and always computes the checksum in software. csum_start: finding the real offset requires parsing to the transport header. Do not add a parser, use existing segmentation parsing. Thanks to SKB_GSO_DODGY, that also catches bad packets that are hw offloaded. Again test both TSO and USO. Do not test UFO for the above reason, and do not test UDP tunnel offload. GSO packet are almost always CHECKSUM_PARTIAL. USO packets may be CHECKSUM_NONE since commit 10154db ("udp: Allow GSO transmit from devices with no checksum offload"), but then still these fields are initialized correctly in udp4_hwcsum/udp6_hwcsum_outgoing. So no need to test for ip_summed == CHECKSUM_PARTIAL first. This revises an existing fix mentioned in the Fixes tag, which broke small packets with GSO offload, as detected by kselftests. Link: https://syzkaller.appspot.com/bug?extid=e1db31216c789f552871 Link: https://lore.kernel.org/netdev/[email protected] Fixes: e269d79 ("net: missing check virtio") Cc: [email protected] Signed-off-by: Willem de Bruijn <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent a7f3abc commit 89add40

File tree

3 files changed

+12
-11
lines changed

3 files changed

+12
-11
lines changed

include/linux/virtio_net.h

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
5656
unsigned int thlen = 0;
5757
unsigned int p_off = 0;
5858
unsigned int ip_proto;
59-
u64 ret, remainder, gso_size;
6059

6160
if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
6261
switch (hdr->gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
@@ -99,16 +98,6 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
9998
u32 off = __virtio16_to_cpu(little_endian, hdr->csum_offset);
10099
u32 needed = start + max_t(u32, thlen, off + sizeof(__sum16));
101100

102-
if (hdr->gso_size) {
103-
gso_size = __virtio16_to_cpu(little_endian, hdr->gso_size);
104-
ret = div64_u64_rem(skb->len, gso_size, &remainder);
105-
if (!(ret && (hdr->gso_size > needed) &&
106-
((remainder > needed) || (remainder == 0)))) {
107-
return -EINVAL;
108-
}
109-
skb_shinfo(skb)->tx_flags |= SKBFL_SHARED_FRAG;
110-
}
111-
112101
if (!pskb_may_pull(skb, needed))
113102
return -EINVAL;
114103

@@ -182,6 +171,11 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
182171
if (gso_type != SKB_GSO_UDP_L4)
183172
return -EINVAL;
184173
break;
174+
case SKB_GSO_TCPV4:
175+
case SKB_GSO_TCPV6:
176+
if (skb->csum_offset != offsetof(struct tcphdr, check))
177+
return -EINVAL;
178+
break;
185179
}
186180

187181
/* Kernel has a special handling for GSO_BY_FRAGS. */

net/ipv4/tcp_offload.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,9 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb,
140140
if (thlen < sizeof(*th))
141141
goto out;
142142

143+
if (unlikely(skb_checksum_start(skb) != skb_transport_header(skb)))
144+
goto out;
145+
143146
if (!pskb_may_pull(skb, thlen))
144147
goto out;
145148

net/ipv4/udp_offload.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,10 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
278278
if (gso_skb->len <= sizeof(*uh) + mss)
279279
return ERR_PTR(-EINVAL);
280280

281+
if (unlikely(skb_checksum_start(gso_skb) !=
282+
skb_transport_header(gso_skb)))
283+
return ERR_PTR(-EINVAL);
284+
281285
if (skb_gso_ok(gso_skb, features | NETIF_F_GSO_ROBUST)) {
282286
/* Packet is from an untrusted source, reset gso_segs. */
283287
skb_shinfo(gso_skb)->gso_segs = DIV_ROUND_UP(gso_skb->len - sizeof(*uh),

0 commit comments

Comments
 (0)