Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d44403ec authored by Mohamed Khalfella's avatar Mohamed Khalfella Committed by Greg Kroah-Hartman
Browse files

skbuff: skb_segment, Call zero copy functions before using skbuff frags 



commit 2ea35288c83b3d501a88bc17f2df8f176b5cc96f upstream.

Commit bf5c25d6 ("skbuff: in skb_segment, call zerocopy functions
once per nskb") added the call to zero copy functions in skb_segment().
The change introduced a bug in skb_segment() because skb_orphan_frags()
may possibly change the number of fragments or allocate new fragments
altogether leaving nrfrags and frag to point to the old values. This can
cause a panic with stacktrace like the one below.

[  193.894380] BUG: kernel NULL pointer dereference, address: 00000000000000bc
[  193.895273] CPU: 13 PID: 18164 Comm: vh-net-17428 Kdump: loaded Tainted: G           O      5.15.123+ #26
[  193.903919] RIP: 0010:skb_segment+0xb0e/0x12f0
[  194.021892] Call Trace:
[  194.027422]  <TASK>
[  194.072861]  tcp_gso_segment+0x107/0x540
[  194.082031]  inet_gso_segment+0x15c/0x3d0
[  194.090783]  skb_mac_gso_segment+0x9f/0x110
[  194.095016]  __skb_gso_segment+0xc1/0x190
[  194.103131]  netem_enqueue+0x290/0xb10 [sch_netem]
[  194.107071]  dev_qdisc_enqueue+0x16/0x70
[  194.110884]  __dev_queue_xmit+0x63b/0xb30
[  194.121670]  bond_start_xmit+0x159/0x380 [bonding]
[  194.128506]  dev_hard_start_xmit+0xc3/0x1e0
[  194.131787]  __dev_queue_xmit+0x8a0/0xb30
[  194.138225]  macvlan_start_xmit+0x4f/0x100 [macvlan]
[  194.141477]  dev_hard_start_xmit+0xc3/0x1e0
[  194.144622]  sch_direct_xmit+0xe3/0x280
[  194.147748]  __dev_queue_xmit+0x54a/0xb30
[  194.154131]  tap_get_user+0x2a8/0x9c0 [tap]
[  194.157358]  tap_sendmsg+0x52/0x8e0 [tap]
[  194.167049]  handle_tx_zerocopy+0x14e/0x4c0 [vhost_net]
[  194.173631]  handle_tx+0xcd/0xe0 [vhost_net]
[  194.176959]  vhost_worker+0x76/0xb0 [vhost]
[  194.183667]  kthread+0x118/0x140
[  194.190358]  ret_from_fork+0x1f/0x30
[  194.193670]  </TASK>

In this case calling skb_orphan_frags() updated nr_frags leaving nrfrags
local variable in skb_segment() stale. This resulted in the code hitting
i >= nrfrags prematurely and trying to move to next frag_skb using
list_skb pointer, which was NULL, and caused kernel panic. Move the call
to zero copy functions before using frags and nr_frags.

Fixes: bf5c25d6 ("skbuff: in skb_segment, call zerocopy functions once per nskb")
Signed-off-by: default avatarMohamed Khalfella <mkhalfella@purestorage.com>
Reported-by: default avatarAmit Goyal <agoyal@purestorage.com>
Cc: stable@vger.kernel.org
Reviewed-by: default avatarEric Dumazet <edumazet@google.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 64831fb6

net/core/skbuff.c

+20 −14
Original line number Diff line number Diff line
@@ -3686,21 +3686,20 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, 
	struct sk_buff *segs = NULL;

	struct sk_buff *tail = NULL;

	struct sk_buff *list_skb = skb_shinfo(head_skb)->frag_list;

	skb_frag_t *frag = skb_shinfo(head_skb)->frags;

	unsigned int mss = skb_shinfo(head_skb)->gso_size;

	unsigned int doffset = head_skb->data - skb_mac_header(head_skb);

	struct sk_buff *frag_skb = head_skb;

	unsigned int offset = doffset;

	unsigned int tnl_hlen = skb_tnl_header_len(head_skb);

	unsigned int partial_segs = 0;

	unsigned int headroom;

	unsigned int len = head_skb->len;

	struct sk_buff *frag_skb;

	skb_frag_t *frag;

	__be16 proto;

	bool csum, sg;

	int nfrags = skb_shinfo(head_skb)->nr_frags;

	int err = -ENOMEM;

	int i = 0;

	int pos;

	int nfrags, pos;

	int dummy;



	if ((skb_shinfo(head_skb)->gso_type & SKB_GSO_DODGY) &&
@@ -3778,6 +3777,13 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, 
	headroom = skb_headroom(head_skb);

	pos = skb_headlen(head_skb);



	if (skb_orphan_frags(head_skb, GFP_ATOMIC))

		return ERR_PTR(-ENOMEM);



	nfrags = skb_shinfo(head_skb)->nr_frags;

	frag = skb_shinfo(head_skb)->frags;

	frag_skb = head_skb;



	do {

		struct sk_buff *nskb;

		skb_frag_t *nskb_frag;
@@ -3802,6 +3808,10 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, 
		    (skb_headlen(list_skb) == len || sg)) {

			BUG_ON(skb_headlen(list_skb) > len);



			nskb = skb_clone(list_skb, GFP_ATOMIC);

			if (unlikely(!nskb))

				goto err;



			i = 0;

			nfrags = skb_shinfo(list_skb)->nr_frags;

			frag = skb_shinfo(list_skb)->frags;
@@ -3820,12 +3830,8 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, 
				frag++;

			}



			nskb = skb_clone(list_skb, GFP_ATOMIC);

			list_skb = list_skb->next;



			if (unlikely(!nskb))

				goto err;



			if (unlikely(pskb_trim(nskb, len))) {

				kfree_skb(nskb);

				goto err;
@@ -3890,12 +3896,16 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, 
		skb_shinfo(nskb)->tx_flags |= skb_shinfo(head_skb)->tx_flags &

					      SKBTX_SHARED_FRAG;



		if (skb_orphan_frags(frag_skb, GFP_ATOMIC) ||

		    skb_zerocopy_clone(nskb, frag_skb, GFP_ATOMIC))

		if (skb_zerocopy_clone(nskb, frag_skb, GFP_ATOMIC))

			goto err;



		while (pos < offset + len) {

			if (i >= nfrags) {

				if (skb_orphan_frags(list_skb, GFP_ATOMIC) ||

				    skb_zerocopy_clone(nskb, list_skb,

						       GFP_ATOMIC))

					goto err;



				i = 0;

				nfrags = skb_shinfo(list_skb)->nr_frags;

				frag = skb_shinfo(list_skb)->frags;
@@ -3909,10 +3919,6 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, 
					i--;

					frag--;

				}

				if (skb_orphan_frags(frag_skb, GFP_ATOMIC) ||

				    skb_zerocopy_clone(nskb, frag_skb,

						       GFP_ATOMIC))

					goto err;



				list_skb = list_skb->next;

			}