[PATCH 08/21] exec: Fix latent file_ram_alloc() error handling bug

Markus Armbruster posted 21 patches 6 years, 2 months ago
Maintainers: David Gibson <david@gibson.dropbear.id.au>, Igor Mammedov <imammedo@redhat.com>, Corey Minyard <minyard@acm.org>, Richard Henderson <rth@twiddle.net>, Jason Wang <jasowang@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Michael Roth <mdroth@linux.vnet.ibm.com>, Markus Armbruster <armbru@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, "Michael S. Tsirkin" <mst@redhat.com>, David Hildenbrand <david@redhat.com>, Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>, Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Paul Burton <pburton@wavecomp.com>, Christian Borntraeger <borntraeger@de.ibm.com>
[PATCH 08/21] exec: Fix latent file_ram_alloc() error handling bug
Posted by Markus Armbruster 6 years, 2 months ago
When os_mem_prealloc() fails, file_ram_alloc() calls qemu_ram_munmap()
and returns null.  Except it doesn't when its @errp argument is null,
because it checks for failure with (errp && *errp).  Messed up in
commit 056b68af77 "fix qemu exit on memory hotplug when allocation
fails at prealloc time".

The bug can't bite as no caller actually passes null.  Fix it anyway.

Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 exec.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index ffdb518535..45695a5f2d 100644
--- a/exec.c
+++ b/exec.c
@@ -1841,6 +1841,7 @@ static void *file_ram_alloc(RAMBlock *block,
                             bool truncate,
                             Error **errp)
 {
+    Error *err = NULL;
     MachineState *ms = MACHINE(qdev_get_machine());
     void *area;
 
@@ -1898,8 +1899,9 @@ static void *file_ram_alloc(RAMBlock *block,
     }
 
     if (mem_prealloc) {
-        os_mem_prealloc(fd, area, memory, ms->smp.cpus, errp);
-        if (errp && *errp) {
+        os_mem_prealloc(fd, area, memory, ms->smp.cpus, &err);
+        if (err) {
+            error_propagate(errp, err);
             qemu_ram_munmap(fd, area, memory);
             return NULL;
         }
-- 
2.21.0


Re: [PATCH 08/21] exec: Fix latent file_ram_alloc() error handling bug
Posted by Igor Mammedov 6 years, 2 months ago
On Sat, 30 Nov 2019 20:42:27 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> When os_mem_prealloc() fails, file_ram_alloc() calls qemu_ram_munmap()
> and returns null.  Except it doesn't when its @errp argument is null,
> because it checks for failure with (errp && *errp).  Messed up in
> commit 056b68af77 "fix qemu exit on memory hotplug when allocation
> fails at prealloc time".
> 
> The bug can't bite as no caller actually passes null.  Fix it anyway.
> 
> Cc: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  exec.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index ffdb518535..45695a5f2d 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1841,6 +1841,7 @@ static void *file_ram_alloc(RAMBlock *block,
>                              bool truncate,
>                              Error **errp)
>  {
> +    Error *err = NULL;
>      MachineState *ms = MACHINE(qdev_get_machine());
>      void *area;
>  
> @@ -1898,8 +1899,9 @@ static void *file_ram_alloc(RAMBlock *block,
>      }
>  
>      if (mem_prealloc) {
> -        os_mem_prealloc(fd, area, memory, ms->smp.cpus, errp);
> -        if (errp && *errp) {
> +        os_mem_prealloc(fd, area, memory, ms->smp.cpus, &err);
> +        if (err) {
> +            error_propagate(errp, err);
>              qemu_ram_munmap(fd, area, memory);
>              return NULL;
>          }