[PATCH 5/5] hw/nvme: Fix bootindex suffix use-after-free

Akihiko Odaki posted 5 patches 2 weeks, 2 days ago
Maintainers: Viktor Prutyanov <viktor.prutyanov@phystech.edu>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Keith Busch <kbusch@kernel.org>, Klaus Jensen <its@irrelevant.dk>, Jesper Devantier <foss@defmacro.it>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>
[PATCH 5/5] hw/nvme: Fix bootindex suffix use-after-free
Posted by Akihiko Odaki 2 weeks, 2 days ago
The bootindex suffix can be used as long as the property is alive.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/nvme/nvme.h | 1 +
 hw/nvme/ns.c   | 7 +++----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 8f8c78c85036..d66f7dc82d5c 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -239,6 +239,7 @@ typedef struct NvmeNamespace {
     DeviceState  parent_obj;
     BlockConf    blkconf;
     int32_t      bootindex;
+    char         bootindex_suffix[24];
     int64_t      size;
     int64_t      moff;
     NvmeIdNs     id_ns;
diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 58800b3414a3..38f86a17268f 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -944,12 +944,11 @@ static void nvme_ns_class_init(ObjectClass *oc, const void *data)
 static void nvme_ns_instance_init(Object *obj)
 {
     NvmeNamespace *ns = NVME_NS(obj);
-    char *bootindex = g_strdup_printf("/namespace@%d,0", ns->params.nsid);
 
-    device_add_bootindex_property(obj, &ns->bootindex, "bootindex",
-                                  bootindex, DEVICE(obj));
+    sprintf(ns->bootindex_suffix, "/namespace@%" PRIu32 ",0", ns->params.nsid);
 
-    g_free(bootindex);
+    device_add_bootindex_property(obj, &ns->bootindex, "bootindex",
+                                  ns->bootindex_suffix, DEVICE(obj));
 }
 
 static const TypeInfo nvme_ns_info = {

-- 
2.52.0
Re: [PATCH 5/5] hw/nvme: Fix bootindex suffix use-after-free
Posted by Michael Tokarev 1 week ago
On 1/25/26 09:42, Akihiko Odaki wrote:
> The bootindex suffix can be used as long as the property is alive.

This looks like a qemu-stable material.

Please let me know if it isn't.

Thanks,

/mjt
Re: [PATCH 5/5] hw/nvme: Fix bootindex suffix use-after-free
Posted by Philippe Mathieu-Daudé 1 week ago
On 3/2/26 08:59, Michael Tokarev wrote:
> On 1/25/26 09:42, Akihiko Odaki wrote:
>> The bootindex suffix can be used as long as the property is alive.
> 
> This looks like a qemu-stable material.

Correct, sorry for missing the @qemu-stable tag.

> 
> Please let me know if it isn't.
> 
> Thanks,
> 
> /mjt
>
Re: [PATCH 5/5] hw/nvme: Fix bootindex suffix use-after-free
Posted by Philippe Mathieu-Daudé 2 weeks, 2 days ago
On 25/1/26 07:42, Akihiko Odaki wrote:
> The bootindex suffix can be used as long as the property is alive.

True, so:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

But maybe it shouldn't (we should g_strdup it like other QOM fields).

> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   hw/nvme/nvme.h | 1 +
>   hw/nvme/ns.c   | 7 +++----
>   2 files changed, 4 insertions(+), 4 deletions(-)

Re: [PATCH 5/5] hw/nvme: Fix bootindex suffix use-after-free
Posted by Klaus Jensen 1 week, 5 days ago
On Jan 25 17:56, Philippe Mathieu-Daudé wrote:
> On 25/1/26 07:42, Akihiko Odaki wrote:
> > The bootindex suffix can be used as long as the property is alive.
> 
> True, so:
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> But maybe it shouldn't (we should g_strdup it like other QOM fields).
> 

Do you mean that device_add_bootindex_property() should g_strdup the
suffix? And then free it in property_release_bootindex()? That would
make sense to me.

My grepping tells me that nvme-ns is the only device where the suffix is
dynamic. In all other places it is a literal.
Re: [PATCH 5/5] hw/nvme: Fix bootindex suffix use-after-free
Posted by Keith Busch 1 week, 3 days ago
On Wed, Jan 28, 2026 at 10:23:11AM -0800, Klaus Jensen wrote:
> On Jan 25 17:56, Philippe Mathieu-Daudé wrote:
> > On 25/1/26 07:42, Akihiko Odaki wrote:
> > > The bootindex suffix can be used as long as the property is alive.
> > 
> > True, so:
> > 
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > 
> > But maybe it shouldn't (we should g_strdup it like other QOM fields).
> > 
> 
> Do you mean that device_add_bootindex_property() should g_strdup the
> suffix? And then free it in property_release_bootindex()? That would
> make sense to me.

Doesn't it already do that? 

  device_add_bootindex_property
    object_property_add
      object_property_try_add
        g_strdup

?
Re: [PATCH 5/5] hw/nvme: Fix bootindex suffix use-after-free
Posted by Keith Busch 1 week, 3 days ago
On Fri, Jan 30, 2026 at 06:37:11PM -0700, Keith Busch wrote:
> On Wed, Jan 28, 2026 at 10:23:11AM -0800, Klaus Jensen wrote:
> > On Jan 25 17:56, Philippe Mathieu-Daudé wrote:
> > > On 25/1/26 07:42, Akihiko Odaki wrote:
> > > > The bootindex suffix can be used as long as the property is alive.
> > > 
> > > True, so:
> > > 
> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > 
> > > But maybe it shouldn't (we should g_strdup it like other QOM fields).
> > > 
> > 
> > Do you mean that device_add_bootindex_property() should g_strdup the
> > suffix? And then free it in property_release_bootindex()? That would
> > make sense to me.
> 
> Doesn't it already do that? 
> 
>   device_add_bootindex_property
>     object_property_add
>       object_property_try_add
>         g_strdup
> 
> ?

Sorry, nevermind. I was tracking the "name", not the "suffix", in which
case I agree with Klaus. If only for consistency between the two "const
char *" parameters.