WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [GIT] xen: netback: take net_schedule_list_lock when removin

To: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Subject: [Xen-devel] [GIT] xen: netback: take net_schedule_list_lock when removing entry from net_schedule_list
From: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
Date: Fri, 15 Oct 2010 13:46:10 +0100
Cc: Tomasz Wroblewski <tomasz.wroblewski@xxxxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Fri, 15 Oct 2010 05:47:05 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: Citrix Systems, Inc.
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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@xxxxxxxxxx>
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@xxxxxxxxxx>
Cc: Tomasz Wroblewski <tomasz.wroblewski@xxxxxxxxxx>
---
 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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [GIT] xen: netback: take net_schedule_list_lock when removing entry from net_schedule_list, Ian Campbell <=