WARNING - OLD ARCHIVES

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

xen-devel

Re: [Xen-devel] [PATCH] xenbus: fix possible crash in xenbus_uevent_back

To: Jan Beulich <JBeulich@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xenbus: fix possible crash in xenbus_uevent_backend
From: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Date: Mon, 18 Jul 2011 14:26:38 +0100
Cc: Olaf Hering <olaf@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 18 Jul 2011 06:27:29 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4E244F6B020000780004DED5@xxxxxxxxxxxxxxxxxxxx>
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>
Organization: Citrix Systems, Inc.
References: <20110718124059.GA7893@xxxxxxxxx> <1310994684.20648.47.camel@xxxxxxxxxxxxxxxxxxxxxx> <4E244F6B020000780004DED5@xxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On Mon, 2011-07-18 at 14:21 +0100, Jan Beulich wrote:
> >>> 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().

So it is, didn't spot that.

> > 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.

Oh, right. I'll argue that that's the parameter to container_of
de-referencing, not the macro itself, to make myself look less dumb ;-P

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel