Patchwork [v2,1/1] qom/object: Don't poll cast cache for NULL objects

login
register
mail settings
Submitter Peter Crosthwaite
Date May 22, 2013, 1:19 a.m.
Message ID <8e2bef6a55753869c50bfa32226f7fcf0439ca62.1369183592.git.peter.crosthwaite@xilinx.com>
Download mbox | patch
Permalink /patch/245476/
State New
Headers show

Comments

Peter Crosthwaite - May 22, 2013, 1:19 a.m.
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

object_dynamic_cast_assert used to be tolerant of NULL objects and not
assert. It's clear from the implementation that this is the expected
behavior.

The preceding check of the cast cache dereferences obj however causing
a segfault. Fix by conditionalizing the cast cache logic on obj being
non-null.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Andreas Färber <afaerber@suse.de>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
---
Changed from v1: Fixed 2 commit msg typos (AF review)

 qom/object.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Paolo Bonzini - May 22, 2013, 6:04 a.m.
----- Messaggio originale -----
> Da: "peter crosthwaite" <peter.crosthwaite@xilinx.com>
> A: qemu-devel@nongnu.org
> Cc: aliguori@us.ibm.com, "edgar iglesias" <edgar.iglesias@gmail.com>, pbonzini@redhat.com, afaerber@suse.de
> Inviato: Mercoledì, 22 maggio 2013 3:19:16
> Oggetto: [PATCH v2 1/1] qom/object: Don't poll cast cache for NULL objects
> 
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> 
> object_dynamic_cast_assert used to be tolerant of NULL objects and not
> assert. It's clear from the implementation that this is the expected
> behavior.
> 
> The preceding check of the cast cache dereferences obj however causing
> a segfault. Fix by conditionalizing the cast cache logic on obj being
> non-null.
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
> Changed from v1: Fixed 2 commit msg typos (AF review)
> 
>  qom/object.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index ec88231..803b94b 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -442,7 +442,7 @@ Object *object_dynamic_cast_assert(Object *obj, const
> char *typename,
>      int i;
>      Object *inst;
>  
> -    for (i = 0; i < OBJECT_CLASS_CAST_CACHE; i++) {
> +    for (i = 0; obj && i < OBJECT_CLASS_CAST_CACHE; i++) {
>          if (obj->class->cast_cache[i] == typename) {
>              goto out;
>          }
> @@ -458,7 +458,7 @@ Object *object_dynamic_cast_assert(Object *obj, const
> char *typename,
>  
>      assert(obj == inst);
>  
> -    if (obj == inst) {
> +    if (obj && obj == inst) {
>          for (i = 1; i < OBJECT_CLASS_CAST_CACHE; i++) {
>              obj->class->cast_cache[i - 1] = obj->class->cast_cache[i];
>          }
> --
> 1.8.3.rc1.44.gb387c77.dirty

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

... and added qemu-stable@nongnu.org since this got in pretty close to a release.

Paolo
Edgar Iglesias - May 22, 2013, 6:16 a.m.
On Wed, May 22, 2013 at 11:19:16AM +1000, peter.crosthwaite@xilinx.com wrote:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> 
> object_dynamic_cast_assert used to be tolerant of NULL objects and not
> assert. It's clear from the implementation that this is the expected
> behavior.
> 
> The preceding check of the cast cache dereferences obj however causing
> a segfault. Fix by conditionalizing the cast cache logic on obj being
> non-null.
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>


> ---
> Changed from v1: Fixed 2 commit msg typos (AF review)
> 
>  qom/object.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index ec88231..803b94b 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -442,7 +442,7 @@ Object *object_dynamic_cast_assert(Object *obj, const char *typename,
>      int i;
>      Object *inst;
>  
> -    for (i = 0; i < OBJECT_CLASS_CAST_CACHE; i++) {
> +    for (i = 0; obj && i < OBJECT_CLASS_CAST_CACHE; i++) {
>          if (obj->class->cast_cache[i] == typename) {
>              goto out;
>          }
> @@ -458,7 +458,7 @@ Object *object_dynamic_cast_assert(Object *obj, const char *typename,
>  
>      assert(obj == inst);
>  
> -    if (obj == inst) {
> +    if (obj && obj == inst) {
>          for (i = 1; i < OBJECT_CLASS_CAST_CACHE; i++) {
>              obj->class->cast_cache[i - 1] = obj->class->cast_cache[i];
>          }
> -- 
> 1.8.3.rc1.44.gb387c77.dirty
>
Anthony Liguori - May 22, 2013, 10:58 p.m.
Applied.  Thanks.

Regards,

Anthony Liguori

Patch

diff --git a/qom/object.c b/qom/object.c
index ec88231..803b94b 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -442,7 +442,7 @@  Object *object_dynamic_cast_assert(Object *obj, const char *typename,
     int i;
     Object *inst;
 
-    for (i = 0; i < OBJECT_CLASS_CAST_CACHE; i++) {
+    for (i = 0; obj && i < OBJECT_CLASS_CAST_CACHE; i++) {
         if (obj->class->cast_cache[i] == typename) {
             goto out;
         }
@@ -458,7 +458,7 @@  Object *object_dynamic_cast_assert(Object *obj, const char *typename,
 
     assert(obj == inst);
 
-    if (obj == inst) {
+    if (obj && obj == inst) {
         for (i = 1; i < OBJECT_CLASS_CAST_CACHE; i++) {
             obj->class->cast_cache[i - 1] = obj->class->cast_cache[i];
         }