Patchwork sysbus: Guard against NULL SysBusDevice::init fn

login
register
mail settings
Submitter Peter Crosthwaite
Date March 3, 2013, 5:30 a.m.
Message ID <1362288656-29628-1-git-send-email-peter.crosthwaite@xilinx.com>
Download mbox | patch
Permalink /patch/224531/
State New
Headers show

Comments

Peter Crosthwaite - March 3, 2013, 5:30 a.m.
In certain cases a device model can init with neither a Device::realize or
SysBusDevice::init (i.e. when its possible to do everything in Object::init).
In this case, the device model should be able to leave both SysBusDevice::init
and Device::realize as NULL. However what happens in this case in SysBus's
default Device::realize function will try and call SysBusDevice::init without
checking if it actually exists. A segfault ensues.

Fix by guarding the call to SysBusDevice::init against a NULL pointer. If no
pointer is defined return 0 without action.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/sysbus.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Andreas Färber - March 3, 2013, 1:19 p.m.
Am 03.03.2013 06:30, schrieb Peter Crosthwaite:
> In certain cases a device model can init with neither a Device::realize or
> SysBusDevice::init (i.e. when its possible to do everything in Object::init).
> In this case, the device model should be able to leave both SysBusDevice::init
> and Device::realize as NULL. However what happens in this case in SysBus's
> default Device::realize function will try and call SysBusDevice::init without
> checking if it actually exists. A segfault ensues.
> 
> Fix by guarding the call to SysBusDevice::init against a NULL pointer. If no
> pointer is defined return 0 without action.
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

While the patch is correct, I believe PMM already posted one using an
extra if two weeks ago as part of some series...

Andreas

> ---
>  hw/sysbus.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/sysbus.c b/hw/sysbus.c
> index 6d9d1df..72b309a 100644
> --- a/hw/sysbus.c
> +++ b/hw/sysbus.c
> @@ -118,7 +118,7 @@ static int sysbus_device_init(DeviceState *dev)
>      SysBusDevice *sd = SYS_BUS_DEVICE(dev);
>      SysBusDeviceClass *sbc = SYS_BUS_DEVICE_GET_CLASS(sd);
>  
> -    return sbc->init(sd);
> +    return sbc->init ? sbc->init(sd) : 0;
>  }
>  
>  DeviceState *sysbus_create_varargs(const char *name,
>
Peter Maydell - March 3, 2013, 1:27 p.m.
On 3 March 2013 21:19, Andreas Färber <afaerber@suse.de> wrote:
> Am 03.03.2013 06:30, schrieb Peter Crosthwaite:
>> Fix by guarding the call to SysBusDevice::init against a NULL pointer. If no
>> pointer is defined return 0 without action.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> While the patch is correct, I believe PMM already posted one using an
> extra if two weeks ago as part of some series...

Yep, http://patchwork.ozlabs.org/patch/222985/

-- PMM

Patch

diff --git a/hw/sysbus.c b/hw/sysbus.c
index 6d9d1df..72b309a 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -118,7 +118,7 @@  static int sysbus_device_init(DeviceState *dev)
     SysBusDevice *sd = SYS_BUS_DEVICE(dev);
     SysBusDeviceClass *sbc = SYS_BUS_DEVICE_GET_CLASS(sd);
 
-    return sbc->init(sd);
+    return sbc->init ? sbc->init(sd) : 0;
 }
 
 DeviceState *sysbus_create_varargs(const char *name,