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] [PATCH linux-2.6.18-xen] netfront: plug a theoretical leak o

To: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH linux-2.6.18-xen] netfront: plug a theoretical leak on setup_device()'s error path
From: Laszlo Ersek <lersek@xxxxxxxxxx>
Date: Tue, 24 May 2011 11:38:13 +0200
Delivery-date: Tue, 24 May 2011 02:38:19 -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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110428 Fedora/3.1.10-1.fc14 Lightning/1.0b3pre Mnenhy/0.8.3 Thunderbird/3.1.10
This patch intends to fix a theoretical leak in
netfront::setup_device(), on the error path.

Suppose the frontend is in XenbusStateInitialising and the backend
advances to XenbusStateInitWait. backend_changed() will call
network_connect() -> talk_to_backend() -> setup_device().

If bind_listening_port_to_irqhandler() fails (due to event channel
allocation failure, dynamic IRQ allocation failure etc), then the grant
references and the shared ring pages remain allocated. The error
percolates back to backend_changed() without any cleanup.
backend_changed() will not change the frontend state; it will stay in
XenbusStateInitialising. If the backend retries XenbusStateInitWait
(possibly through a fake, no-op mediate state, like XenbusStateClosed),
then the frontend repeats the above call chain and leaks the previously
unreleased ring pages and grant references.

If setup_device() fails to grant access to the RX page before it tries
to call bind_listening_port_to_irqhandler(), then only the TX page /
grant reference are leaked in the next round.

I'm not sure what happens when the RX page allocation fails:
xenbus_dev_fatal() is called then, which changes the frontend's state
and seems to preclude a second immediate call to network_connect(). I
think the patch below shouldn't hurt in that case either (or after
earlier failures in setup_device() for that matter).

Thanks for considering.

Signed-off-by: Laszlo Ersek <lersek@xxxxxxxxxx>
---
 drivers/xen/netfront/netfront.c |   18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff -r 415a9b435fef drivers/xen/netfront/netfront.c
--- a/drivers/xen/netfront/netfront.c   Mon May 23 18:36:33 2011 +0100
+++ b/drivers/xen/netfront/netfront.c   Tue May 24 11:37:12 2011 +0200
@@ -215,6 +215,7 @@ static int setup_device(struct xenbus_de
 static struct net_device *create_netdev(struct xenbus_device *);
 
 static void end_access(int, void *);
+static void netif_release_rings(struct netfront_info *);
 static void netif_disconnect_backend(struct netfront_info *);
 
 static int network_connect(struct net_device *);
@@ -522,6 +523,7 @@ static int setup_device(struct xenbus_de
        return 0;
 
  fail:
+       netif_release_rings(info);
        return err;
 }
 
@@ -2152,6 +2154,15 @@ static struct notifier_block notifier_in
 };
 #endif
 
+static void netif_release_rings(struct netfront_info *info)
+{
+       end_access(info->tx_ring_ref, info->tx.sring);
+       end_access(info->rx_ring_ref, info->rx.sring);
+       info->tx_ring_ref = GRANT_INVALID_REF;
+       info->rx_ring_ref = GRANT_INVALID_REF;
+       info->tx.sring = NULL;
+       info->rx.sring = NULL;
+}
 
 static void netif_disconnect_backend(struct netfront_info *info)
 {
@@ -2166,12 +2177,7 @@ static void netif_disconnect_backend(str
                unbind_from_irqhandler(info->irq, info->netdev);
        info->irq = 0;
 
-       end_access(info->tx_ring_ref, info->tx.sring);
-       end_access(info->rx_ring_ref, info->rx.sring);
-       info->tx_ring_ref = GRANT_INVALID_REF;
-       info->rx_ring_ref = GRANT_INVALID_REF;
-       info->tx.sring = NULL;
-       info->rx.sring = NULL;
+       netif_release_rings(info);
 }
 
 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [PATCH linux-2.6.18-xen] netfront: plug a theoretical leak on setup_device()'s error path, Laszlo Ersek <=