[Qemu-devel] [PATCH 0/2] qom: add error handler for security

[Qemu-devel] [PATCH 0/2] qom: add error handler for security

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

The PATCH 2 is splited from another patch series,  which has been
reviewed by Paolo. PATCH 1 can avoid possible leaking memory.

Gonglei (2):
  qom: add error handler for object_property_print()
  qom: add error handler for object alias property

 qom/object.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH 1/2] qom: add error handler for object_property_print()

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Avoid the caller of object_property_print() leaking string
argument's memory, such as qdev_print_props() when
encounter errors.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 qom/object.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index da0919a..74779e6 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1010,11 +1010,19 @@ char *object_property_print(Object *obj, const char *name, bool human,
                             Error **errp)
 {
     StringOutputVisitor *mo;
-    char *string;
+    char *string = NULL;
+    Error *local_err = NULL;
 
     mo = string_output_visitor_new(human);
-    object_property_get(obj, string_output_get_visitor(mo), name, errp);
+    object_property_get(obj, string_output_get_visitor(mo), name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto fail;
+    }
+
     string = string_output_get_string(mo);
+
+fail:
     string_output_visitor_cleanup(mo);
     return string;
 }
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 1/2] qom: add error handler for object_property_print()

[Andreas Färber]

Hi,

Am 23.09.2014 um 05:25 schrieb arei.gonglei@huawei.com:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> Avoid the caller of object_property_print() leaking string
> argument's memory, such as qdev_print_props() when
> encounter errors.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  qom/object.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index da0919a..74779e6 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1010,11 +1010,19 @@ char *object_property_print(Object *obj, const char *name, bool human,
>                              Error **errp)
>  {
>      StringOutputVisitor *mo;
> -    char *string;
> +    char *string = NULL;
> +    Error *local_err = NULL;
>  
>      mo = string_output_visitor_new(human);
> -    object_property_get(obj, string_output_get_visitor(mo), name, errp);
> +    object_property_get(obj, string_output_get_visitor(mo), name, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        goto fail;
> +    }
> +
>      string = string_output_get_string(mo);
> +
> +fail:

Permission to rename the label to "out" as in 2/2?

Series looks good otherwise.

Regards,
Andreas

>      string_output_visitor_cleanup(mo);
>      return string;
>  }
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 1/2] qom: add error handler for object_property_print()

[Gonglei (Arei)]

> Subject: Re: [Qemu-devel] [PATCH 1/2] qom: add error handler for
> object_property_print()
> 
> Hi,
> 
> Am 23.09.2014 um 05:25 schrieb arei.gonglei@huawei.com:
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> > Avoid the caller of object_property_print() leaking string
> > argument's memory, such as qdev_print_props() when
> > encounter errors.
> >
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> >  qom/object.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/qom/object.c b/qom/object.c
> > index da0919a..74779e6 100644
> > --- a/qom/object.c
> > +++ b/qom/object.c
> > @@ -1010,11 +1010,19 @@ char *object_property_print(Object *obj, const
> char *name, bool human,
> >                              Error **errp)
> >  {
> >      StringOutputVisitor *mo;
> > -    char *string;
> > +    char *string = NULL;
> > +    Error *local_err = NULL;
> >
> >      mo = string_output_visitor_new(human);
> > -    object_property_get(obj, string_output_get_visitor(mo), name, errp);
> > +    object_property_get(obj, string_output_get_visitor(mo), name,
> &local_err);
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> > +        goto fail;
> > +    }
> > +
> >      string = string_output_get_string(mo);
> > +
> > +fail:
> 
> Permission to rename the label to "out" as in 2/2?
> 
OK. Thanks. Will do. :)

Best regards,
-Gonglei

> Series looks good otherwise.
> 
> Regards,
> Andreas
> 
> >      string_output_visitor_cleanup(mo);
> >      return string;
> >  }
> >
> 
> 
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH 2/2] qom: add error handler for object alias property

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

object_property_add_alias() is called at some
places at present. And its parameter errp may not NULL,
such as
 object_property_add_alias(obj, "iothread", OBJECT(&dev->vdev),"iothread",
                              &error_abort);
This patch add error handler for security.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qom/object.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index 74779e6..81542fb 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1642,6 +1642,7 @@ void object_property_add_alias(Object *obj, const char *name,
     ObjectProperty *op;
     ObjectProperty *target_prop;
     gchar *prop_type;
+    Error *local_err = NULL;
 
     target_prop = object_property_find(target_obj, target_name, errp);
     if (!target_prop) {
@@ -1663,9 +1664,15 @@ void object_property_add_alias(Object *obj, const char *name,
                              property_get_alias,
                              property_set_alias,
                              property_release_alias,
-                             prop, errp);
+                             prop, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        g_free(prop);
+        goto out;
+    }
     op->resolve = property_resolve_alias;
 
+out:
     g_free(prop_type);
 }
 
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 0/2] qom: add error handler for security

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 414 bytes --]

On 09/22/2014 09:25 PM, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> The PATCH 2 is splited from another patch series,  which has been

s/splited/split/

'split' is one of those stupid English words whose past tense is spelled
identically to its present tense.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH 0/2] qom: add error handler for security

[Gonglei (Arei)]

> Subject: Re: [Qemu-devel] [PATCH 0/2] qom: add error handler for security
> 
> On 09/22/2014 09:25 PM, arei.gonglei@huawei.com wrote:
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> > The PATCH 2 is splited from another patch series,  which has been
> 
> s/splited/split/
> 
> 'split' is one of those stupid English words whose past tense is spelled
> identically to its present tense.
> 
Hmm.. Thanks!

Best regards,
-Gonglei

> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org

Re: [Qemu-devel] [PATCH 0/2] qom: add error handler for security

[Paolo Bonzini]

Il 23/09/2014 05:25, arei.gonglei@huawei.com ha scritto:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> The PATCH 2 is splited from another patch series,  which has been
> reviewed by Paolo. PATCH 1 can avoid possible leaking memory.
> 
> Gonglei (2):
>   qom: add error handler for object_property_print()
>   qom: add error handler for object alias property
> 
>  qom/object.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Andreas, can you queue it in qom-next?

Paolo

Re: [Qemu-devel] [PATCH 0/2] qom: add error handler for security

[Gonglei (Arei)]

> >
> > The PATCH 2 is splited from another patch series,  which has been
> > reviewed by Paolo. PATCH 1 can avoid possible leaking memory.
> >
> > Gonglei (2):
> >   qom: add error handler for object_property_print()
> >   qom: add error handler for object alias property
> >
> >  qom/object.c | 21 ++++++++++++++++++---
> >  1 file changed, 18 insertions(+), 3 deletions(-)
> >
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> 

Thanks, Paolo. I merged those two patches in my next patch series
 [PATCH 0/7] add description field in ObjectProperty and PropertyInfo struct
because of depending. Maybe I should spilt them again?

Best regards,
-Gonglei

> Andreas, can you queue it in qom-next?
> 
> Paolo

Re: [Qemu-devel] [PATCH 0/2] qom: add error handler for security

[Paolo Bonzini]

Il 23/09/2014 15:21, Gonglei (Arei) ha scritto:
> Thanks, Paolo. I merged those two patches in my next patch series
>  [PATCH 0/7] add description field in ObjectProperty and PropertyInfo struct
> because of depending. Maybe I should spilt them again?

No, don't worry.  Maintainers will handle it just fine.

Paolo

Re: [Qemu-devel] [PATCH 0/2] qom: add error handler for security

[Gonglei (Arei)]

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: Tuesday, September 23, 2014 9:23 PM
> Subject: Re: [PATCH 0/2] qom: add error handler for security
> 
> Il 23/09/2014 15:21, Gonglei (Arei) ha scritto:
> > Thanks, Paolo. I merged those two patches in my next patch series
> >  [PATCH 0/7] add description field in ObjectProperty and PropertyInfo struct
> > because of depending. Maybe I should spilt them again?
> 
> No, don't worry.  Maintainers will handle it just fine.
> 
> Paolo

OK , Thanks. :)

Best regards,
-Gonglei