1. Patchew
  2. QEMU
  3. [PATCH v3 0/6] igvm: Supply MADT via IGVM parameter
  4. View patch

[PATCH v3 3/6] igvm: Add missing NULL check

Oliver Steffen posted 6 patches 1 month ago
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, Ani Sinha <anisinha@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Marcelo Tosatti <mtosatti@redhat.com>, Zhao Liu <zhao1.liu@intel.com>
There is a newer version of this series
[PATCH v3 3/6] igvm: Add missing NULL check
Posted by Oliver Steffen 1 month ago 
Check for NULL pointer returned from igvm_get_buffer().
Documentation for that function calls for that unconditionally.

Signed-off-by: Oliver Steffen <osteffen@redhat.com>
---
 backends/igvm.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/backends/igvm.c b/backends/igvm.c
index a350c890cc..dc1fd026cb 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -170,9 +170,16 @@ static int qigvm_handler(QIgvm *ctx, uint32_t type, Error **errp)
                 (int)header_handle);
             return -1;
         }
-        header_data = igvm_get_buffer(ctx->file, header_handle) +
-                      sizeof(IGVM_VHS_VARIABLE_HEADER);
-        result = handlers[handler].handler(ctx, header_data, errp);
+        header_data = igvm_get_buffer(ctx->file, header_handle);
+        if (header_data == NULL) {
+            error_setg(
+                errp,
+                "IGVM: Failed to get directive header data (code: %d)",
+                (int)header_handle);
+            result = -1;
+        } else {
+            result = handlers[handler].handler(ctx, header_data + sizeof(IGVM_VHS_VARIABLE_HEADER), errp);
+        }
         igvm_free_buffer(ctx->file, header_handle);
         return result;
     }
-- 
2.52.0
Re: [PATCH v3 3/6] igvm: Add missing NULL check
Posted by Ani Sinha 3 weeks, 6 days ago 

> On 9 Jan 2026, at 8:04 PM, Oliver Steffen <osteffen@redhat.com> wrote:
> 
> Check for NULL pointer returned from igvm_get_buffer().
> Documentation for that function calls for that unconditionally.
> 
> Signed-off-by: Oliver Steffen <osteffen@redhat.com>
> ---
> backends/igvm.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/backends/igvm.c b/backends/igvm.c
> index a350c890cc..dc1fd026cb 100644
> --- a/backends/igvm.c
> +++ b/backends/igvm.c
> @@ -170,9 +170,16 @@ static int qigvm_handler(QIgvm *ctx, uint32_t type, Error **errp)
>                 (int)header_handle);
>             return -1;
>         }
> -        header_data = igvm_get_buffer(ctx->file, header_handle) +
> -                      sizeof(IGVM_VHS_VARIABLE_HEADER);
> -        result = handlers[handler].handler(ctx, header_data, errp);
> +        header_data = igvm_get_buffer(ctx->file, header_handle);
> +        if (header_data == NULL) {
> +            error_setg(
> +                errp,
> +                "IGVM: Failed to get directive header data (code: %d)",
> +                (int)header_handle);
> +            result = -1;

I would just return -1 here and remove the else {} clause below. It makes it slightly easier to follow the code.

> +        } else {
> +            result = handlers[handler].handler(ctx, header_data + sizeof(IGVM_VHS_VARIABLE_HEADER), errp);
> +        }
>         igvm_free_buffer(ctx->file, header_handle);
>         return result;
>     }
> -- 
> 2.52.0
>
Re: [PATCH v3 3/6] igvm: Add missing NULL check
Posted by Oliver Steffen 3 weeks, 6 days ago 
On Tue, Jan 13, 2026 at 8:21 AM Ani Sinha <anisinha@redhat.com> wrote:
>
>
>
> > On 9 Jan 2026, at 8:04 PM, Oliver Steffen <osteffen@redhat.com> wrote:
> >
> > Check for NULL pointer returned from igvm_get_buffer().
> > Documentation for that function calls for that unconditionally.
> >
> > Signed-off-by: Oliver Steffen <osteffen@redhat.com>
> > ---
> > backends/igvm.c | 13 ++++++++++---
> > 1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/backends/igvm.c b/backends/igvm.c
> > index a350c890cc..dc1fd026cb 100644
> > --- a/backends/igvm.c
> > +++ b/backends/igvm.c
> > @@ -170,9 +170,16 @@ static int qigvm_handler(QIgvm *ctx, uint32_t type, Error **errp)
> >                 (int)header_handle);
> >             return -1;
> >         }
> > -        header_data = igvm_get_buffer(ctx->file, header_handle) +
> > -                      sizeof(IGVM_VHS_VARIABLE_HEADER);
> > -        result = handlers[handler].handler(ctx, header_data, errp);
> > +        header_data = igvm_get_buffer(ctx->file, header_handle);
> > +        if (header_data == NULL) {
> > +            error_setg(
> > +                errp,
> > +                "IGVM: Failed to get directive header data (code: %d)",
> > +                (int)header_handle);
> > +            result = -1;
>
> I would just return -1 here and remove the else {} clause below. It makes it slightly easier to follow the code.

Sure, can do.
But are we ok with publicating
      igvm_free_buffer(ctx->file, header_handle);
which we need before returning?

>
> > +        } else {
> > +            result = handlers[handler].handler(ctx, header_data + sizeof(IGVM_VHS_VARIABLE_HEADER), errp);
> > +        }
> >         igvm_free_buffer(ctx->file, header_handle);
> >         return result;
> >     }
> > --
> > 2.52.0
> >
>
Re: [PATCH v3 3/6] igvm: Add missing NULL check
Posted by Luigi Leonardi 1 month ago 
On Fri, Jan 09, 2026 at 03:34:10PM +0100, Oliver Steffen wrote:
>Check for NULL pointer returned from igvm_get_buffer().
>Documentation for that function calls for that unconditionally.
>
>Signed-off-by: Oliver Steffen <osteffen@redhat.com>
>---
> backends/igvm.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
>diff --git a/backends/igvm.c b/backends/igvm.c
>index a350c890cc..dc1fd026cb 100644
>--- a/backends/igvm.c
>+++ b/backends/igvm.c
>@@ -170,9 +170,16 @@ static int qigvm_handler(QIgvm *ctx, uint32_t type, Error **errp)
>                 (int)header_handle);
>             return -1;
>         }
>-        header_data = igvm_get_buffer(ctx->file, header_handle) +
>-                      sizeof(IGVM_VHS_VARIABLE_HEADER);
>-        result = handlers[handler].handler(ctx, header_data, errp);
>+        header_data = igvm_get_buffer(ctx->file, header_handle);
>+        if (header_data == NULL) {
>+            error_setg(
>+                errp,
>+                "IGVM: Failed to get directive header data (code: %d)",
>+                (int)header_handle);
>+            result = -1;
>+        } else {
>+            result = handlers[handler].handler(ctx, header_data + sizeof(IGVM_VHS_VARIABLE_HEADER), errp);
>+        }
>         igvm_free_buffer(ctx->file, header_handle);
>         return result;
>     }
>-- 2.52.0
>

IMHO this should be sent a separate patch with the Fixes tag as you are
fixing a bug.

Luigi
Re: [PATCH v3 3/6] igvm: Add missing NULL check
Posted by Gerd Hoffmann 4 weeks ago 
On Fri, Jan 09, 2026 at 06:37:04PM +0100, Luigi Leonardi wrote:
> On Fri, Jan 09, 2026 at 03:34:10PM +0100, Oliver Steffen wrote:
> >Check for NULL pointer returned from igvm_get_buffer().
> >Documentation for that function calls for that unconditionally.
> >
> >Signed-off-by: Oliver Steffen <osteffen@redhat.com>
> >---
> > backends/igvm.c | 13 ++++++++++---
> > 1 file changed, 10 insertions(+), 3 deletions(-)
> >
> >diff --git a/backends/igvm.c b/backends/igvm.c
> >index a350c890cc..dc1fd026cb 100644
> >--- a/backends/igvm.c
> >+++ b/backends/igvm.c
> >@@ -170,9 +170,16 @@ static int qigvm_handler(QIgvm *ctx, uint32_t type, Error **errp)
> >                 (int)header_handle);
> >             return -1;
> >         }
> >-        header_data = igvm_get_buffer(ctx->file, header_handle) +
> >-                      sizeof(IGVM_VHS_VARIABLE_HEADER);
> >-        result = handlers[handler].handler(ctx, header_data, errp);
> >+        header_data = igvm_get_buffer(ctx->file, header_handle);
> >+        if (header_data == NULL) {
> >+            error_setg(
> >+                errp,
> >+                "IGVM: Failed to get directive header data (code: %d)",
> >+                (int)header_handle);
> >+            result = -1;
> >+        } else {
> >+            result = handlers[handler].handler(ctx, header_data + sizeof(IGVM_VHS_VARIABLE_HEADER), errp);
> >+        }
> >         igvm_free_buffer(ctx->file, header_handle);
> >         return result;
> >     }
> >-- 2.52.0
> >
> 
> IMHO this should be sent a separate patch

Huh?  It /is/ a separate patch ...

> with the Fixes tag as you are
> fixing a bug.

That makes sense indeed.

take care,
  Gerd
Re: [PATCH v3 3/6] igvm: Add missing NULL check
Posted by Luigi Leonardi 4 weeks ago 
On Mon, Jan 12, 2026 at 10:41:01AM +0100, Gerd Hoffmann wrote:
>On Fri, Jan 09, 2026 at 06:37:04PM +0100, Luigi Leonardi wrote:
>> On Fri, Jan 09, 2026 at 03:34:10PM +0100, Oliver Steffen wrote:
>> >Check for NULL pointer returned from igvm_get_buffer().
>> >Documentation for that function calls for that unconditionally.
>> >
>> >Signed-off-by: Oliver Steffen <osteffen@redhat.com>
>> >---
>> > backends/igvm.c | 13 ++++++++++---
>> > 1 file changed, 10 insertions(+), 3 deletions(-)
>> >
>> >diff --git a/backends/igvm.c b/backends/igvm.c
>> >index a350c890cc..dc1fd026cb 100644
>> >--- a/backends/igvm.c
>> >+++ b/backends/igvm.c
>> >@@ -170,9 +170,16 @@ static int qigvm_handler(QIgvm *ctx, uint32_t type, Error **errp)
>> >                 (int)header_handle);
>> >             return -1;
>> >         }
>> >-        header_data = igvm_get_buffer(ctx->file, header_handle) +
>> >-                      sizeof(IGVM_VHS_VARIABLE_HEADER);
>> >-        result = handlers[handler].handler(ctx, header_data, errp);
>> >+        header_data = igvm_get_buffer(ctx->file, header_handle);
>> >+        if (header_data == NULL) {
>> >+            error_setg(
>> >+                errp,
>> >+                "IGVM: Failed to get directive header data (code: %d)",
>> >+                (int)header_handle);
>> >+            result = -1;
>> >+        } else {
>> >+            result = handlers[handler].handler(ctx, header_data + sizeof(IGVM_VHS_VARIABLE_HEADER), errp);
>> >+        }
>> >         igvm_free_buffer(ctx->file, header_handle);
>> >         return result;
>> >     }
>> >-- 2.52.0
>> >
>>
>> IMHO this should be sent a separate patch
>
>Huh?  It /is/ a separate patch ...

Sorry, I meant outside of this series.

Luigi
Re: [PATCH v3 3/6] igvm: Add missing NULL check
Posted by Gerd Hoffmann 4 weeks ago 
  Hi,

> > > IMHO this should be sent a separate patch
> > 
> > Huh?  It /is/ a separate patch ...
> 
> Sorry, I meant outside of this series.

Not needed, separate patch is good enough, even though sending a
separate 'fixes' series might make sense in some cases (split an
already long patch series, or during freeze where only fixes are
allowed before the next release).

take care,
  Gerd
Re: [PATCH v3 3/6] igvm: Add missing NULL check
Posted by Oliver Steffen 3 weeks, 6 days ago 
On Mon, Jan 12, 2026 at 10:57 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > > > IMHO this should be sent a separate patch
> > >
> > > Huh?  It /is/ a separate patch ...
> >
> > Sorry, I meant outside of this series.
>
> Not needed, separate patch is good enough, even though sending a
> separate 'fixes' series might make sense in some cases (split an
> already long patch series, or during freeze where only fixes are
> allowed before the next release).

Since there are more identical cases of missing NULL checks here,
I'd take this patch out of this series and handle all instances together
in a new patch/series.

Thanks
- Olvier

>
> take care,
>   Gerd
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.