Re: [Qemu-devel] [PATCH] qdev: fix use-after-free in the error path of qdev_init_nofail

[Anthony Liguori]

On 06/27/2012 08:14 AM, Andreas Färber wrote:
> Am 27.06.2012 14:41, schrieb Anthony Liguori:
> >  From Markus:
> >
> > Before:
> >
> >      $ qemu-system-x86_64 -display none -drive if=ide
> >      qemu-system-x86_64: Device needs media, but drive is empty
> >      qemu-system-x86_64: Initialization of device ide-hd failed
> >      [Exit 1 ]
> >
> > After:
> >
> >      $ qemu-system-x86_64 -display none -drive if=ide
> >      qemu-system-x86_64: Device needs media, but drive is empty
> >      Segmentation fault (core dumped)
> >      [Exit 139 (SIGSEGV)]
> >
> > This error always existed as qdev_init() frees the object.  But QOM
> > goes a bit further and purposefully sets the class pointer to NULL to
> > help find use-after-free.  It worked :-)
> >
> > Cc: Andreas Faerber<address@hidden>
> > Reported-by: Markus Armbruster<address@hidden>
> > Signed-off-by: Anthony Liguori<address@hidden>
>
> Reviewed-by: Andreas Färber<address@hidden>
>
> This together with the semantics discussions we're having makes me think
> we should attack "QOM'ifying" qdev sooner than later. I.e., reviewing
> what naming, chaining, etc. we can already change to align the
> TYPE_DEVICE-derived types with the generic QOM infrastructure.

We really ought to find all uses of qdev_init_nofail() or qdev_try_init() 
(including the sysbus et al derivatives) and add explicit qdev_free() calls in 
the error handling case such that we can remove the qdev_free() calls that are 
done automatically in the init function.

Destructing an object automagically in a virtual method is pretty darn evil and 
really promotes these sort of problems.

Regards,

Anthony Liguori

> Andreas