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/
Home Products Support Community News


Re: [Xen-devel] [PATCH] xen/netback: Fix null-pointer access in netback_

To: "Bastian Blank" <waldi@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xen/netback: Fix null-pointer access in netback_uevent
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Mon, 31 May 2010 08:37:43 +0100
Cc: Jeremy Fitzhardinge <jeremy@xxxxxxxx>, James Harper <james.harper@xxxxxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Mon, 31 May 2010 00:38:34 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20100529184452.GA18365@xxxxxxxxxxxxxxxxxxxxxxx>
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>
References: <20100527165558.GA11358@xxxxxxxxxxxxxxxxxxxxxxx> <AEC6C66638C05B468B556EA548C1A77D01996BD8@trantor> <AEC6C66638C05B468B556EA548C1A77D01996BDB@trantor> <AEC6C66638C05B468B556EA548C1A77D01996BE0@trantor> <20100528090325.GA13575@xxxxxxxxxxxxxxxxxxxxxxx> <4BFFFD30.6000807@xxxxxxxx> <AEC6C66638C05B468B556EA548C1A77D01996C1F@trantor> <4C005228.5060107@xxxxxxxx> <AEC6C66638C05B468B556EA548C1A77D01996C20@trantor> <20100529064411.GA12168@xxxxxxxxxxxxxxxxxxxxxxx> <20100529184452.GA18365@xxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
>>> On 29.05.10 at 20:44, Bastian Blank <waldi@xxxxxxxxxx> wrote:
> The uevent method of Xen netback does not check if the the network
> device is already setup and tries to dereference a null-pointer it not.
> Signed-off-by: Bastian Blank <waldi@xxxxxxxxxx>
> ---
>  drivers/xen/netback/xenbus.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
> index 70636d0..88262bb 100644
> --- a/drivers/xen/netback/xenbus.c
> +++ b/drivers/xen/netback/xenbus.c
> @@ -163,7 +163,6 @@ fail:
>  static int netback_uevent(struct xenbus_device *xdev, struct 
> kobj_uevent_env *env)
>  {
>       struct backend_info *be = dev_get_drvdata(&xdev->dev);
> -     struct xen_netif *netif = be->netif;
>       char *val;
>       DPRINTK("netback_uevent");
> @@ -182,7 +181,7 @@ static int netback_uevent(struct xenbus_device *xdev, 
> struct kobj_uevent_env *en
>               kfree(val);
>       }
> -     if (add_uevent_var(env, "vif=%s", netif->dev->name))
> +     if (be && be->netif && add_uevent_var(env, "vif=%s", 
> be->netif->dev->name))
>               return -ENOMEM;
>       return 0;

Unfortunately this still seems incomplete: Just checking be->netif to
be non-NULL isn't sufficient, as the sysfs access may race backend
teardown afaics. Hence proper serialization is going to be needed for
that case.

Furthermore, the backend creation patch also needs adjustment,
as it currently stores a non-NULL non-pointer value in be->netif if
netif_alloc() fails. To require the sysfs path to use IS_ERR() on
be->netif, I think netif_alloc()'s result should be stored to a local 
variable first and only written to be->netif when valid.


Xen-devel mailing list