Re: [PATCH v3 06/49] RAMBlock: Add support of KVM private guest memfd

[Paolo Bonzini <pbonzini@xxxxxxxxxx>]

On 3/20/24 09:39, Michael Roth wrote:
> @@ -1842,6 +1842,17 @@ static void ram_block_add(RAMBlock *new_block, Error **errp)
>           }
>       }
>   
> +    if (kvm_enabled() && (new_block->flags & RAM_GUEST_MEMFD)) {
> +        assert(new_block->guest_memfd < 0);
> +
> +        new_block->guest_memfd = kvm_create_guest_memfd(new_block->max_length,
> +                                                        0, errp);
> +        if (new_block->guest_memfd < 0) {
> +            qemu_mutex_unlock_ramlist();
> +            return;
> +        }
> +    }
> +

This potentially leaks new_block->host.  This can be squashed into the patch:

diff --git a/system/physmem.c b/system/physmem.c
index 3a4a3f10d5a..0836aff190e 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -1810,6 +1810,7 @@ static void ram_block_add(RAMBlock *new_block, Error **errp)
     const bool shared = qemu_ram_is_shared(new_block);
     RAMBlock *block;
     RAMBlock *last_block = NULL;
+    bool free_on_error = false;
     ram_addr_t old_ram_size, new_ram_size;
     Error *err = NULL;
 
@@ -1839,6 +1841,7 @@ static void ram_block_add(RAMBlock *new_block, Error **errp)
                 return;
             }
             memory_try_enable_merging(new_block->host, new_block->max_length);
+            free_on_error = true;
         }
     }
 
@@ -1849,7 +1852,7 @@ static void ram_block_add(RAMBlock *new_block, Error **errp)
                                                         0, errp);
         if (new_block->guest_memfd < 0) {
             qemu_mutex_unlock_ramlist();
-            return;
+            goto out_free;
         }
     }
 
@@ -1901,6 +1904,13 @@ static void ram_block_add(RAMBlock *new_block, Error **errp)
         ram_block_notify_add(new_block->host, new_block->used_length,
                              new_block->max_length);
     }
+    return;
+
+out_free:
+    if (free_on_error) {
+        qemu_anon_ram_free(new_block->host, new_block->max_length);
+        new_block->host = NULL;
+    }
 }
 
 #ifdef CONFIG_POSIX