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>, "Ian Campbell" <Ian.Campbell@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xenbus: fix possible crash in xenbus_uevent_backend
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Mon, 18 Jul 2011 14:21:15 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 18 Jul 2011 06:21:59 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1310994684.20648.47.camel@xxxxxxxxxxxxxxxxxxxxxx>
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> <1310994684.20648.47.camel@xxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
>>> On 18.07.11 at 15:11, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Mon, 2011-07-18 at 13:40 +0100, Olaf Hering 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)
>>              return -ENODEV;
> Is this fixing an actual crash which you observed of just something you
> noticed looking at the code?
> container_of is pure pointer arithmetic without dereferencing so to get
> bus == NULL you'd need xdev == offsetof(struct xen_bus_type, bus) or
> some such.

-offsetof(struct xen_bus_type, bus)

> I think the check of xdev is correct, although it might be clearer if it

Not really, as it similarly is the result of a container_of().

> preceded the "bus = ... " it's not actively harmful where it is since
> container_of doesn't dereference the pointer.

Doesn't? "xdev->dev.bus" very much looks like a de-reference to me.


Xen-devel mailing list