Ian Campbell
2010-Oct-15 12:46 UTC
[Xen-devel] [GIT] xen: netback: take net_schedule_list_lock when removing entry from net_schedule_list
The following changes since commit adb236eb839264255954076dbc4fc40f5738f08e:
Ian Campbell (1):
xen: netback: increase size of rx_meta array.
are available in the git repository at:
git://xenbits.xen.org/people/ianc/linux-2.6.git for-jeremy/netback
Ian Campbell (1):
xen: netback: take net_schedule_list_lock when removing entry from
net_schedule_list
drivers/xen/netback/netback.c | 35 ++++++++++++++++++++++++++++-------
1 files changed, 28 insertions(+), 7 deletions(-)
Ian.
>From 3af2f5d05b1236d11e952152ac1f505e6b0e8935 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Fri, 15 Oct 2010 13:41:44 +0100
Subject: [PATCH] xen: netback: take net_schedule_list_lock when removing entry
from net_schedule_list
There is a race in net_tx_build_mops between checking if
net_schedule_list is empty and actually dequeuing the first entry on
the list. If another thread dequeues the only entry on the list during
this window we crash because list_first_entry expects a non-empty
list, like so:
[ 0.133127] BUG: unable to handle kernel NULL pointer dereference at 00000008
[ 0.133132] IP: [<c12aae71>] net_tx_build_mops+0x91/0xa70
[ 0.133142] *pdpt = 0000000000000000 *pde = 000000000000000f
[ 0.133147] Oops: 0002 1 SMP
[ 0.133150] last sysfs file:
[ 0.133152] Modules linked in:
[ 0.133154]
[ 0.133156] Pid: 55, comm: netback/1 Not tainted (2.6.32.12-0.7.1 #1) Latitude
E4310
[ 0.133158] EIP: 0061:[<c12aae71>] EFLAGS: 00010202 CPU: 1
[ 0.133161] EIP is at net_tx_build_mops+0x91/0xa70
[ 0.133163] EAX: 00000012 EBX: 00000008 ECX: e112b734 EDX: e112b76c
[ 0.133165] ESI: ffffff30 EDI: 00000000 EBP: e112b734 ESP: dfe85d98
[ 0.133167] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0069
[ 0.133169] Process netback/1 (pid: 55, ti=dfe84000 task=dfe83340
task.ti=dfe84000)
[ 0.133170] Stack:
[ 0.133172] 00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000000
[ 0.133177] <0> 00000000 e112b734 e112ec08 e112b7f8 e112ec08 ffffff30
00000000 00000000
[ 0.133186] <0> 00000000 00000000 00000000 e112b76c dfe85df4 00000001
00000000 aaaaaaaa
[ 0.133193] Call Trace:
[ 0.133202] [<c12abc7f>] net_tx_action+0x42f/0xac0
[ 0.133206] [<c12ac37a>] netbk_action_thread+0x6a/0x1b0
[ 0.133212] [<c1057444>] kthread+0x74/0x80
[ 0.133218] [<c10049d7>] kernel_thread_helper+0x7/0x10
[ 0.133220] Code: c4 00 00 00 89 74 24 58 39 74 24 2c 0f 84 c7 06 00 00 8b 74 24
\
58 8b 5c 24 58 81 ee d0 00 00 00 83 c3 08 89 74 24 34 8b 7c 24
\
58 <f0> ff 47 08 89 f0 e8 b4 f9 ff ff 8b 46 2c 8b 56 34 89 44
24 5c
[ 0.133261] EIP: [<c12aae71>] net_tx_build_mops+0x91/0xa70 SS:ESP
0069:dfe85d98
[ 0.133265] CR2: 0000000000000008
[ 0.133274] --[ end trace e2c5c15f54bd9d93 ]--
Therefore after the initial lock free check for an empty list check
again with the lock held before dequeueing the entry.
Based on a patch by Tomasz Wroblewski.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Tomasz Wroblewski <tomasz.wroblewski@citrix.com>
---
drivers/xen/netback/netback.c | 35 ++++++++++++++++++++++++++++-------
1 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index 9052895..c448675 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -785,15 +785,34 @@ static int __on_net_schedule_list(struct xen_netif *netif)
return !list_empty(&netif->list);
}
+/* Must be called with net_schedule_list_lock held */
static void remove_from_net_schedule_list(struct xen_netif *netif)
{
- struct xen_netbk *netbk = &xen_netbk[netif->group];
- spin_lock_irq(&netbk->net_schedule_list_lock);
if (likely(__on_net_schedule_list(netif))) {
list_del_init(&netif->list);
netif_put(netif);
}
+}
+
+static struct xen_netif *poll_net_schedule_list(struct xen_netbk *netbk)
+{
+ struct xen_netif *netif = NULL;
+
+ spin_lock_irq(&netbk->net_schedule_list_lock);
+ if (list_empty(&netbk->net_schedule_list))
+ goto out;
+
+ netif = list_first_entry(&netbk->net_schedule_list,
+ struct xen_netif, list);
+ if (!netif)
+ goto out;
+
+ netif_get(netif);
+
+ remove_from_net_schedule_list(netif);
+out:
spin_unlock_irq(&netbk->net_schedule_list_lock);
+ return netif;
}
static void add_to_net_schedule_list_tail(struct xen_netif *netif)
@@ -828,7 +847,10 @@ void netif_schedule_work(struct xen_netif *netif)
void netif_deschedule_work(struct xen_netif *netif)
{
+ struct xen_netbk *netbk = &xen_netbk[netif->group];
+ spin_lock_irq(&netbk->net_schedule_list_lock);
remove_from_net_schedule_list(netif);
+ spin_unlock_irq(&netbk->net_schedule_list_lock);
}
@@ -1312,12 +1334,11 @@ static unsigned net_tx_build_mops(struct xen_netbk
*netbk)
int work_to_do;
unsigned int data_len;
pending_ring_idx_t index;
-
+
/* Get a netif from the list with work to do. */
- netif = list_first_entry(&netbk->net_schedule_list,
- struct xen_netif, list);
- netif_get(netif);
- remove_from_net_schedule_list(netif);
+ netif = poll_net_schedule_list(netbk);
+ if (!netif)
+ continue;
RING_FINAL_CHECK_FOR_REQUESTS(&netif->tx, work_to_do);
if (!work_to_do) {
--
1.5.6.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel