From: Kyle Zeng tcp_gro_receive() excludes empty non-GSO packets with the unsigned len - 1 check, but its GSO branch only checks that gso_size matches the held packet's MSS. A trusted TCP GSO skb can reach GRO again with no payload after TC pedit increases its TCP data offset. If the skb is fully linear, skb_gro_receive() then enters its fragment transfer path with nr_frags == 0. The do-while loop copies skb_frag_t entries backwards beyond both fragment arrays. An unprivileged process using its own user and network namespaces can trigger a KASAN slab-out-of-bounds write and panic through veth, pedit and GRE. More generally, the payload of such a packet can be shrunk to any value, making it inconsistent with its gso_segs, and a GSO packet with gso_segs == 0 contributes no segment to the GRO packet. Do not aggregate GSO packets with gso_segs == 0, and require the payload of GSO packets to be consistent with gso_segs and gso_size, the last segment being allowed to be short. This rejects the empty skb before either TCP merge helper, and covers both IPv4 and IPv6. Fixes: 5eddb24901ee ("gro: add support of (hw)gro packets to gro stack") Signed-off-by: Kyle Zeng [edumazet: check payload against gso_segs, refuse gso_segs == 0] Signed-off-by: Eric Dumazet --- net/core/gro.c | 5 +++-- net/ipv4/tcp_offload.c | 12 +++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/net/core/gro.c b/net/core/gro.c index ff828a3dce3ca649e149a840bd6e3dfd9ddaf0c4..1b033855a20db6b8b996179f0b81e38393d3864a 100644 --- a/net/core/gro.c +++ b/net/core/gro.c @@ -513,9 +513,10 @@ static enum gro_result dev_gro_receive(struct gro_node *gro, NAPI_GRO_CB(skb)->count = 1; if (unlikely(skb_is_gso(skb))) { NAPI_GRO_CB(skb)->count = skb_shinfo(skb)->gso_segs; - /* Only support TCP and non DODGY users. */ + /* Only support TCP and non DODGY users, with a sane gso_segs. */ if (!skb_is_gso_tcp(skb) || - (skb_shinfo(skb)->gso_type & SKB_GSO_DODGY)) + (skb_shinfo(skb)->gso_type & SKB_GSO_DODGY) || + !NAPI_GRO_CB(skb)->count) NAPI_GRO_CB(skb)->flush = 1; } diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c index e74d99ca9face6ecb853e928b538bd66a377038c..b84fcd07c7230fc3230fdbc3a79d68d1e79b058e 100644 --- a/net/ipv4/tcp_offload.c +++ b/net/ipv4/tcp_offload.c @@ -315,14 +315,20 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb, mss = skb_shinfo(p)->gso_size; - /* If skb is a GRO packet, make sure its gso_size matches prior packet mss. + /* If skb is a GRO packet, make sure its gso_size matches prior packet + * mss, and that its payload is consistent with its gso_segs + * (the last segment can be short). * If it is a single frame, do not aggregate it if its length * is bigger than our mss. */ - if (unlikely(skb_is_gso(skb))) + if (unlikely(skb_is_gso(skb))) { + unsigned int segs = NAPI_GRO_CB(skb)->count; + flush |= (mss != skb_shinfo(skb)->gso_size); - else + flush |= len <= (segs - 1) * mss || len > segs * mss; + } else { flush |= (len - 1) >= mss; + } flush |= (ntohl(th2->seq) + skb_gro_len(p)) ^ ntohl(th->seq); flush |= skb_cmp_decrypted(p, skb); -- 2.53.0