From 13cdcd12a2feb7704c1baa8e3825aa7df6e090e1 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 26 May 2011 08:07:05 +0100 Subject: [PATCH] netfront: plug a theoretical leak on setup_device()'s 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 this patch shouldn't hurt in that case either (or after earlier failures in setup_device() for that matter). Signed-off-by: Laszlo Ersek --- drivers/xen/netfront/netfront.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/xen/netfront/netfront.c b/drivers/xen/netfront/netfront.c index 46b79961..f1605355 100644 --- a/drivers/xen/netfront/netfront.c +++ b/drivers/xen/netfront/netfront.c @@ -215,6 +215,7 @@ static int setup_device(struct xenbus_device *, struct netfront_info *); 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_device *dev, struct netfront_info *info) return 0; fail: + netif_release_rings(info); return err; } @@ -2152,6 +2154,15 @@ static struct notifier_block notifier_inetdev = { }; #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(struct netfront_info *info) 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); } -- 2.39.5