Re: [Qemu-devel] [PATCH] qapi: output visitor crashes qemu if it encounters a NULL value

[Marcel Apfelbaum <marcel.a@redhat.com>]

On Mon, 2014-02-17 at 10:38 -0700, Eric Blake wrote:
> On 02/17/2014 04:52 AM, Marcel Apfelbaum wrote:
> > A NULL value is not added to visitor's stack, but there
> > is no check for that when the visitor tries to return
> > that value, leading to Qemu crash.
> 
> Do you have an easy formula for reproducing the crash?

Hi Eric, thank you for your review!

In order to reproduce this you need to use object_property_get_str
on an object with a string property with a null value.

I don't know if in the current code base we have this scenario, but
I am trying to QOMify the QemuMachine and properties as "kernel" may be NULL.

Either way (if NULL properties are not wanted), IMHO it is recommended to cover such cases in order to avoid QEMU crash. 

> 
> > 
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > ---
> >  qapi/qmp-output-visitor.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
> > index 74a5684..0562f49 100644
> > --- a/qapi/qmp-output-visitor.c
> > +++ b/qapi/qmp-output-visitor.c
> > @@ -66,6 +66,11 @@ static QObject *qmp_output_pop(QmpOutputVisitor *qov)
> >  static QObject *qmp_output_first(QmpOutputVisitor *qov)
> >  {
> >      QStackEntry *e = QTAILQ_LAST(&qov->stack, QStack);
> > +
> > +    if (!e) {
> > +        return NULL;
> > +    }
> > +
> 
> The code looks okay to me, but without a formula, my review is fairly weak:
Appreciated,
Marcel

> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
>