Skip to content

Commit 7eab143

Browse files
author
Herton R. Krzesinski
committed
Merge: net: gso: fix panic on frag_list with mixed head alloc types
MR: https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/merge_requests/2001 Bugzilla: https://bugzilla.redhat.com/2166641 commit 9e4b7a9 Author: Jiri Benc <jbenc@redhat.com> Date: Wed Nov 2 17:53:25 2022 +0100 net: gso: fix panic on frag_list with mixed head alloc types Since commit 3dcbdb1 ("net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list"), it is allowed to change gso_size of a GRO packet. However, that commit assumes that "checking the first list_skb member suffices; i.e if either of the list_skb members have non head_frag head, then the first one has too". It turns out this assumption does not hold. We've seen BUG_ON being hit in skb_segment when skbs on the frag_list had differing head_frag with the vmxnet3 driver. This happens because __netdev_alloc_skb and __napi_alloc_skb can return a skb that is page backed or kmalloced depending on the requested size. As the result, the last small skb in the GRO packet can be kmalloced. There are three different locations where this can be fixed: (1) We could check head_frag in GRO and not allow GROing skbs with different head_frag. However, that would lead to performance regression on normal forward paths with unmodified gso_size, where !head_frag in the last packet is not a problem. (2) Set a flag in bpf_skb_net_grow and bpf_skb_net_shrink indicating that NETIF_F_SG is undesirable. That would need to eat a bit in sk_buff. Furthermore, that flag can be unset when all skbs on the frag_list are page backed. To retain good performance, bpf_skb_net_grow/shrink would have to walk the frag_list. (3) Walk the frag_list in skb_segment when determining whether NETIF_F_SG should be cleared. This of course slows things down. This patch implements (3). To limit the performance impact in skb_segment, the list is walked only for skbs with SKB_GSO_DODGY set that have gso_size changed. Normal paths thus will not hit it. We could check only the last skb but since we need to walk the whole list anyway, let's stay on the safe side. Fixes: 3dcbdb1 ("net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list") Signed-off-by: Jiri Benc <jbenc@redhat.com> Reviewed-by: Willem de Bruijn <willemb@google.com> Link: https://lore.kernel.org/r/e04426a6a91baf4d1081e1b478c82b5de25fdf21.1667407944.git.jbenc@redhat.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Jiri Benc <jbenc@redhat.com> Approved-by: Paolo Abeni <pabeni@redhat.com> Approved-by: Hangbin Liu <haliu@redhat.com> Approved-by: Andrea Claudi <aclaudi@redhat.com> Approved-by: Ivan Vecera <ivecera@redhat.com> Signed-off-by: Herton R. Krzesinski <herton@redhat.com>
2 parents d224312 + ffcea31 commit 7eab143

File tree

1 file changed

+19
-17
lines changed

1 file changed

+19
-17
lines changed

net/core/skbuff.c

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4121,23 +4121,25 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
41214121
int i = 0;
41224122
int pos;
41234123

4124-
if (list_skb && !list_skb->head_frag && skb_headlen(list_skb) &&
4125-
(skb_shinfo(head_skb)->gso_type & SKB_GSO_DODGY)) {
4126-
/* gso_size is untrusted, and we have a frag_list with a linear
4127-
* non head_frag head.
4128-
*
4129-
* (we assume checking the first list_skb member suffices;
4130-
* i.e if either of the list_skb members have non head_frag
4131-
* head, then the first one has too).
4132-
*
4133-
* If head_skb's headlen does not fit requested gso_size, it
4134-
* means that the frag_list members do NOT terminate on exact
4135-
* gso_size boundaries. Hence we cannot perform skb_frag_t page
4136-
* sharing. Therefore we must fallback to copying the frag_list
4137-
* skbs; we do so by disabling SG.
4138-
*/
4139-
if (mss != GSO_BY_FRAGS && mss != skb_headlen(head_skb))
4140-
features &= ~NETIF_F_SG;
4124+
if ((skb_shinfo(head_skb)->gso_type & SKB_GSO_DODGY) &&
4125+
mss != GSO_BY_FRAGS && mss != skb_headlen(head_skb)) {
4126+
struct sk_buff *check_skb;
4127+
4128+
for (check_skb = list_skb; check_skb; check_skb = check_skb->next) {
4129+
if (skb_headlen(check_skb) && !check_skb->head_frag) {
4130+
/* gso_size is untrusted, and we have a frag_list with
4131+
* a linear non head_frag item.
4132+
*
4133+
* If head_skb's headlen does not fit requested gso_size,
4134+
* it means that the frag_list members do NOT terminate
4135+
* on exact gso_size boundaries. Hence we cannot perform
4136+
* skb_frag_t page sharing. Therefore we must fallback to
4137+
* copying the frag_list skbs; we do so by disabling SG.
4138+
*/
4139+
features &= ~NETIF_F_SG;
4140+
break;
4141+
}
4142+
}
41414143
}
41424144

41434145
__skb_push(head_skb, doffset);

0 commit comments

Comments
 (0)