netdev - [RFC PATCH 08/13] xen-netback: clone skb if skb->xmit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 12 May 2015 19:18:32 +0200
From:	Joao Martins <joao.martins@...lab.eu>
To:	<xen-devel@...ts.xenproject.org>, <netdev@...r.kernel.org>
CC:	<wei.liu2@...rix.com>, <ian.campbell@...rix.com>,
	<david.vrabel@...rix.com>, <boris.ostrovsky@...cle.com>,
	<konrad.wilk@...cle.com>, Joao Martins <joao.martins@...lab.eu>
Subject: [RFC PATCH 08/13] xen-netback: clone skb if skb->xmit_more is set

On xenvif_start_xmit() we have an additional queue to the netback RX
kthread that will sends the packet. When using burst>1 pktgen sets
skb->xmit_more to tell the driver that there more skbs in the queue.
However, pktgen transmits the same skb <burst> times, which leads to
the BUG below. Long story short adding the same skb in the rx_queue
queue leads to crash. Specifically, having pktgen running with burst=2
what happens is: when we queue the second skb (that is the same as
the first queued skb), the list will have the tail element with skb->prev
which is the skb itself. On skb_unlink (i.e. when dequeueing the skb)
skb->prev will become NULL, but still having list->next pointing to the
unlinked skb. Because of this skb_peek will still return an skb, which
will redo the skb_unlink trying to set (skb->prev)->next where skb->prev
is now NULL, thus leading to the crash (trace below).

I'm not sure what the best way to fix this but since it's only happening
when we use pktgen with burst>1: I chose doing an skb_clone when we don't
use persistent grants and skb->xmit_more flag is set, and when
CONFIG_NET_PKTGEN is compiled builtin.

BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
IP: [<ffffffffa01dbcdc>] xenvif_rx_dequeue+0x7c/0x120 [xen_netback]
PGD 0
Oops: 0002 [#1] SMP
CPU: 1 PID: 10391 Comm: vif510.1-q0-gue Not tainted 4.0.0-rc2-net-next+
task: ffff88003b0ce400 ti: ffff880008538000 task.ti: ffff880008538000
RIP: e030:[<ffffffffa01dbcdc>]  [<ffffffffa01dbcdc>]
xenvif_rx_dequeue+0x7c/0x120 [xen_netback]
RSP: e02b:ffff88000853bde8  EFLAGS: 00010006
RAX: 0000000000000000 RBX: ffffc9000212e000 RCX: 00000000000000e4
RDX: 0000000000000000 RSI: ffff88003b0c0200 RDI: ffffc90002139a24
RBP: ffff88000853bdf8 R08: ffff880008538000 R09: 0000000000000000
R10: aaaaaaaaaaaaaaaa R11: 0000000000000000 R12: ffff8800089a6400
R13: ffffc9000212e000 R14: ffffc90002139a10 R15: 0000000000000000
FS:  0000000000000000(0000) GS:ffff88003f700000(0000)
knlGS:ffff88003f700000
CS:  e033 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000008 CR3: 000000000260b000 CR4: 0000000000042660
Stack:
 ffff88000853be48 ffff88000853be30 ffff88000853beb8 ffffffffa01e19ea
 ffff88000853be60 ffff88003b0ce400 ffff88003ba418c0 ffffc900021399c0
 0000000000000000 ffff88000853be30 ffff88000853be30 ffff000000000000
Call Trace:
 [<ffffffffa01e19ea>] xenvif_kthread_guest_rx+0x26a/0x6e0 [xen_netback]
 [<ffffffffa01e1780>] ? xenvif_map_frontend_rings+0x110/0x110 [xen_netback]
 [<ffffffff8111ae9b>] kthread+0x11b/0x150
 [<ffffffff81120000>] ? clean_sort_range+0x170/0x2f0
 [<ffffffff8111ad80>] ? kthread_stop+0x230/0x230
 [<ffffffff81d6957c>] ret_from_fork+0x7c/0xb0
 [<ffffffff8111ad80>] ? kthread_stop+0x230/0x230
Code: 01 48 83 05 9e f5 00 00 01 49 8b 44 24 08 49 8b 14 24 49 c7 44 24 08
00 00 00 00 49 c7 04 24 00 00 00 00 48 83 05 84 f5 00 00 01 <48> 89 42 08
48 89 10 41 8b 84 24 80 00 00 00 29 83 2c ba 00 00
RIP  [<ffffffffa01dbcdc>] xenvif_rx_dequeue+0x7c/0x120 [xen_netback]
 RSP <ffff88000853bde8>
CR2: 0000000000000008
---[ end trace b3caaf6875c8a975 ]---

Signed-off-by: Joao Martins <joao.martins@...lab.eu>
---
 drivers/net/xen-netback/interface.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index dfe2b7b..5748ba5 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -170,6 +170,15 @@ static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	cb->expires = jiffies + vif->drain_timeout;
 
 	if (!queue->vif->persistent_grants) {
+#ifdef CONFIG_NET_PKTGEN
+		if (skb->xmit_more) {
+			struct sk_buff *nskb;
+
+			nskb = skb_clone(skb, GFP_ATOMIC | __GFP_NOWARN);
+			dev_kfree_skb(skb);
+			skb = nskb;
+		}
+#endif
 		xenvif_rx_queue_tail(queue, skb);
 		xenvif_kick_thread(queue);
 	} else if (xenvif_rx_map(queue, skb)) {
-- 
2.1.3

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html