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] xenbus: fix possible crash in xenbus_uevent_back

To: "Olaf Hering" <olaf@xxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xenbus: fix possible crash in xenbus_uevent_backend
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Mon, 18 Jul 2011 14:18:26 +0100
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Mon, 18 Jul 2011 06:19:25 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20110718124059.GA7893@xxxxxxxxx>
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: <20110718124059.GA7893@xxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
>>> On 18.07.11 at 14:40, Olaf Hering <olaf@xxxxxxxxx> wrote:
> Fix possible NULL pointer crash in xenbus_uevent_backend().
> The variable to check for should probably be bus.
> Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
> Index: linux-3.0-rc7-xen-kexec/drivers/xen/xenbus/xenbus_probe_backend.c
> ===================================================================
> --- linux-3.0-rc7-xen-kexec.orig/drivers/xen/xenbus/xenbus_probe_backend.c
> +++ linux-3.0-rc7-xen-kexec/drivers/xen/xenbus/xenbus_probe_backend.c
> @@ -104,7 +104,7 @@ static int xenbus_uevent_backend(struct
>       xdev = to_xenbus_device(dev);
>       bus = container_of(xdev->dev.bus, struct xen_bus_type, bus);
> -     if (xdev == NULL)
> +     if (bus == NULL)

How can the result of a container_of() be NULL if the passed in
value is in any way meaningful (i.e. valid or NULL)? If any such
check is really necessary, wouldn't you rather want to check
xdev->dev.bus here?

Looking at the code (and its 2.6.18 tree's counterpart
xenbus_uevent_frontend()), I would rather suspect that this
wasn't meant to check bus in the first place, but instead needlessly
tries to check the to_xenbus_device() result, which likewise can't
reasonably be NULL (as that's again the result of a container_of()).


>               return -ENODEV;
>       /* stuff we want to pass to /sbin/hotplug */

Xen-devel mailing list