linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xen-netback: do not report success if xenvif_alloc() fails
@ 2014-11-21 22:56 Alexey Khoroshilov
  2014-11-24 10:00 ` Wei Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Alexey Khoroshilov @ 2014-11-21 22:56 UTC (permalink / raw)
  To: Ian Campbell, Wei Liu
  Cc: Alexey Khoroshilov, xen-devel, netdev, linux-kernel, ldv-project

If xenvif_alloc() failes, netback_probe() reports success as well as
"online" uevent is emitted. It does not make any sense, but it just
misleads users.

The patch implements propagation of error code if xenvif creation fails.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/net/xen-netback/xenbus.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 4e56a27f9689..fab0d4b42f58 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -39,7 +39,7 @@ struct backend_info {
 static int connect_rings(struct backend_info *be, struct xenvif_queue *queue);
 static void connect(struct backend_info *be);
 static int read_xenbus_vif_flags(struct backend_info *be);
-static void backend_create_xenvif(struct backend_info *be);
+static int backend_create_xenvif(struct backend_info *be);
 static void unregister_hotplug_status_watch(struct backend_info *be);
 static void set_backend_state(struct backend_info *be,
 			      enum xenbus_state state);
@@ -352,7 +352,9 @@ static int netback_probe(struct xenbus_device *dev,
 	be->state = XenbusStateInitWait;
 
 	/* This kicks hotplug scripts, so do it immediately. */
-	backend_create_xenvif(be);
+	err = backend_create_xenvif(be);
+	if (err)
+		goto fail;
 
 	return 0;
 
@@ -397,19 +399,19 @@ static int netback_uevent(struct xenbus_device *xdev,
 }
 
 
-static void backend_create_xenvif(struct backend_info *be)
+static int backend_create_xenvif(struct backend_info *be)
 {
 	int err;
 	long handle;
 	struct xenbus_device *dev = be->dev;
 
 	if (be->vif != NULL)
-		return;
+		return 0;
 
 	err = xenbus_scanf(XBT_NIL, dev->nodename, "handle", "%li", &handle);
 	if (err != 1) {
 		xenbus_dev_fatal(dev, err, "reading handle");
-		return;
+		return (err < 0) ? err : -EINVAL;
 	}
 
 	be->vif = xenvif_alloc(&dev->dev, dev->otherend_id, handle);
@@ -417,10 +419,11 @@ static void backend_create_xenvif(struct backend_info *be)
 		err = PTR_ERR(be->vif);
 		be->vif = NULL;
 		xenbus_dev_fatal(dev, err, "creating interface");
-		return;
+		return err;
 	}
 
 	kobject_uevent(&dev->dev.kobj, KOBJ_ONLINE);
+	return 0;
 }
 
 static void backend_disconnect(struct backend_info *be)
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] xen-netback: do not report success if xenvif_alloc() fails
  2014-11-21 22:56 [PATCH] xen-netback: do not report success if xenvif_alloc() fails Alexey Khoroshilov
@ 2014-11-24 10:00 ` Wei Liu
  2014-11-24 10:44   ` Alexey Khoroshilov
  2014-11-24 10:58   ` [PATCH] xen-netback: do not report success if backend_create_xenvif() fails Alexey Khoroshilov
  0 siblings, 2 replies; 6+ messages in thread
From: Wei Liu @ 2014-11-24 10:00 UTC (permalink / raw)
  To: Alexey Khoroshilov
  Cc: Ian Campbell, Wei Liu, xen-devel, netdev, linux-kernel, ldv-project

On Sat, Nov 22, 2014 at 01:56:28AM +0300, Alexey Khoroshilov wrote:
> If xenvif_alloc() failes, netback_probe() reports success as well as
> "online" uevent is emitted. It does not make any sense, but it just

Sorry, I don't follow. KOBJ_ONLINE event is not emitted in the event of
xenvif_alloc fails, is it?

> misleads users.
> 
> The patch implements propagation of error code if xenvif creation fails.
> 

This patch not only implements propagation of error code when xenvif
creation fails, but also when xenbus_scanf fails. You can simply write
"This patch implements propagation of error code for
backend_create_xenvif".

The rest of this patch looks good to me. Can you rewrite commit message
and resubmit, thanks.

Wei.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xen-netback: do not report success if xenvif_alloc() fails
  2014-11-24 10:00 ` Wei Liu
@ 2014-11-24 10:44   ` Alexey Khoroshilov
  2014-11-24 10:58   ` [PATCH] xen-netback: do not report success if backend_create_xenvif() fails Alexey Khoroshilov
  1 sibling, 0 replies; 6+ messages in thread
From: Alexey Khoroshilov @ 2014-11-24 10:44 UTC (permalink / raw)
  To: Wei Liu; +Cc: Ian Campbell, xen-devel, netdev, linux-kernel, ldv-project

On 24.11.2014 13:00, Wei Liu wrote:
> On Sat, Nov 22, 2014 at 01:56:28AM +0300, Alexey Khoroshilov wrote:
>> If xenvif_alloc() failes, netback_probe() reports success as well as
>> "online" uevent is emitted. It does not make any sense, but it just
> Sorry, I don't follow. KOBJ_ONLINE event is not emitted in the event of
> xenvif_alloc fails, is it?
Yes, you are right.
>
>> misleads users.
>>
>> The patch implements propagation of error code if xenvif creation fails.
>>
> This patch not only implements propagation of error code when xenvif
> creation fails, but also when xenbus_scanf fails. You can simply write
> "This patch implements propagation of error code for
> backend_create_xenvif".
>
> The rest of this patch looks good to me. Can you rewrite commit message
> and resubmit, thanks.
Ok.

--
Thank you,
Alexey

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] xen-netback: do not report success if backend_create_xenvif() fails
  2014-11-24 10:00 ` Wei Liu
  2014-11-24 10:44   ` Alexey Khoroshilov
@ 2014-11-24 10:58   ` Alexey Khoroshilov
  2014-11-24 11:03     ` Wei Liu
  2014-11-24 21:14     ` David Miller
  1 sibling, 2 replies; 6+ messages in thread
From: Alexey Khoroshilov @ 2014-11-24 10:58 UTC (permalink / raw)
  To: Wei Liu
  Cc: Alexey Khoroshilov, Ian Campbell, xen-devel, netdev,
	linux-kernel, ldv-project

If xenvif_alloc() or xenbus_scanf() fail in backend_create_xenvif(),
xenbus is left in offline mode but netback_probe() reports success.

The patch implements propagation of error code for backend_create_xenvif().

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/net/xen-netback/xenbus.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 4e56a27f9689..fab0d4b42f58 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -39,7 +39,7 @@ struct backend_info {
 static int connect_rings(struct backend_info *be, struct xenvif_queue *queue);
 static void connect(struct backend_info *be);
 static int read_xenbus_vif_flags(struct backend_info *be);
-static void backend_create_xenvif(struct backend_info *be);
+static int backend_create_xenvif(struct backend_info *be);
 static void unregister_hotplug_status_watch(struct backend_info *be);
 static void set_backend_state(struct backend_info *be,
 			      enum xenbus_state state);
@@ -352,7 +352,9 @@ static int netback_probe(struct xenbus_device *dev,
 	be->state = XenbusStateInitWait;
 
 	/* This kicks hotplug scripts, so do it immediately. */
-	backend_create_xenvif(be);
+	err = backend_create_xenvif(be);
+	if (err)
+		goto fail;
 
 	return 0;
 
@@ -397,19 +399,19 @@ static int netback_uevent(struct xenbus_device *xdev,
 }
 
 
-static void backend_create_xenvif(struct backend_info *be)
+static int backend_create_xenvif(struct backend_info *be)
 {
 	int err;
 	long handle;
 	struct xenbus_device *dev = be->dev;
 
 	if (be->vif != NULL)
-		return;
+		return 0;
 
 	err = xenbus_scanf(XBT_NIL, dev->nodename, "handle", "%li", &handle);
 	if (err != 1) {
 		xenbus_dev_fatal(dev, err, "reading handle");
-		return;
+		return (err < 0) ? err : -EINVAL;
 	}
 
 	be->vif = xenvif_alloc(&dev->dev, dev->otherend_id, handle);
@@ -417,10 +419,11 @@ static void backend_create_xenvif(struct backend_info *be)
 		err = PTR_ERR(be->vif);
 		be->vif = NULL;
 		xenbus_dev_fatal(dev, err, "creating interface");
-		return;
+		return err;
 	}
 
 	kobject_uevent(&dev->dev.kobj, KOBJ_ONLINE);
+	return 0;
 }
 
 static void backend_disconnect(struct backend_info *be)
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] xen-netback: do not report success if backend_create_xenvif() fails
  2014-11-24 10:58   ` [PATCH] xen-netback: do not report success if backend_create_xenvif() fails Alexey Khoroshilov
@ 2014-11-24 11:03     ` Wei Liu
  2014-11-24 21:14     ` David Miller
  1 sibling, 0 replies; 6+ messages in thread
From: Wei Liu @ 2014-11-24 11:03 UTC (permalink / raw)
  To: Alexey Khoroshilov
  Cc: Wei Liu, Ian Campbell, xen-devel, netdev, linux-kernel, ldv-project

On Mon, Nov 24, 2014 at 01:58:00PM +0300, Alexey Khoroshilov wrote:
> If xenvif_alloc() or xenbus_scanf() fail in backend_create_xenvif(),
> xenbus is left in offline mode but netback_probe() reports success.
> 
> The patch implements propagation of error code for backend_create_xenvif().
> 
> Found by Linux Driver Verification project (linuxtesting.org).
> 
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xen-netback: do not report success if backend_create_xenvif() fails
  2014-11-24 10:58   ` [PATCH] xen-netback: do not report success if backend_create_xenvif() fails Alexey Khoroshilov
  2014-11-24 11:03     ` Wei Liu
@ 2014-11-24 21:14     ` David Miller
  1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2014-11-24 21:14 UTC (permalink / raw)
  To: khoroshilov
  Cc: wei.liu2, ian.campbell, xen-devel, netdev, linux-kernel, ldv-project

From: Alexey Khoroshilov <khoroshilov@ispras.ru>
Date: Mon, 24 Nov 2014 13:58:00 +0300

> If xenvif_alloc() or xenbus_scanf() fail in backend_create_xenvif(),
> xenbus is left in offline mode but netback_probe() reports success.
> 
> The patch implements propagation of error code for backend_create_xenvif().
> 
> Found by Linux Driver Verification project (linuxtesting.org).
> 
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>

Applied, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-11-24 21:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-21 22:56 [PATCH] xen-netback: do not report success if xenvif_alloc() fails Alexey Khoroshilov
2014-11-24 10:00 ` Wei Liu
2014-11-24 10:44   ` Alexey Khoroshilov
2014-11-24 10:58   ` [PATCH] xen-netback: do not report success if backend_create_xenvif() fails Alexey Khoroshilov
2014-11-24 11:03     ` Wei Liu
2014-11-24 21:14     ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).