[Qemu-devel] [PATCH 2/3] virtio pmem: remove memdev null check

Pankaj Gupta posted 3 patches 6 years, 7 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>
[Qemu-devel] [PATCH 2/3] virtio pmem: remove memdev null check
Posted by Pankaj Gupta 6 years, 7 months ago
Coverity reports that when we're assigning vi->size we handle the 
"pmem->memdev is NULL" case; but we then pass it into 
object_get_canonical_path(), which unconditionally dereferences it
and will crash if it is NULL. If this pointer can be NULL then we
need to do something else here.

We are removing 'pmem->memdev' null check here as memdev will never
be null in this function.

Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
---
 hw/virtio/virtio-pmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pmem.c b/hw/virtio/virtio-pmem.c
index adbfb603ab..17c196d107 100644
--- a/hw/virtio/virtio-pmem.c
+++ b/hw/virtio/virtio-pmem.c
@@ -134,8 +134,8 @@ static void virtio_pmem_fill_device_info(const VirtIOPMEM *pmem,
                                          VirtioPMEMDeviceInfo *vi)
 {
     vi->memaddr = pmem->start;
-    vi->size = pmem->memdev ? memory_region_size(&pmem->memdev->mr) : 0;
-    vi->memdev = object_get_canonical_path(OBJECT(pmem->memdev));
+    vi->size    = memory_region_size(&pmem->memdev->mr);
+    vi->memdev  = object_get_canonical_path(OBJECT(pmem->memdev));
 }
 
 static MemoryRegion *virtio_pmem_get_memory_region(VirtIOPMEM *pmem,
-- 
2.14.5


Re: [Qemu-devel] [PATCH 2/3] virtio pmem: remove memdev null check
Posted by Cornelia Huck 6 years, 7 months ago
On Fri, 12 Jul 2019 13:05:53 +0530
Pankaj Gupta <pagupta@redhat.com> wrote:

> Coverity reports that when we're assigning vi->size we handle the 
> "pmem->memdev is NULL" case; but we then pass it into 
> object_get_canonical_path(), which unconditionally dereferences it
> and will crash if it is NULL. If this pointer can be NULL then we
> need to do something else here.
> 
> We are removing 'pmem->memdev' null check here as memdev will never
> be null in this function.

Indeed, we'll fail to realize the device if it is NULL.

> 
> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> ---
>  hw/virtio/virtio-pmem.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>