[libvirt] [PATCH] qemu: Check sev capability pointer before using it

Han Han posted 1 patch 5 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180807014205.18588-1-hhan@redhat.com
Test syntax-check passed
src/qemu/qemu_driver.c | 6 ++++++
1 file changed, 6 insertions(+)
[libvirt] [PATCH] qemu: Check sev capability pointer before using it
Posted by Han Han 5 years, 8 months ago
https://bugzilla.redhat.com/show_bug.cgi?id=1612009

Check sev capability pointer in function qemuGetSEVInfoToParams to avoid
null pointer dereferences.

Signed-off-by: Han Han <hhan@redhat.com>
---
 src/qemu/qemu_driver.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fb0d4a8c7a..3daaef586f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -21452,6 +21452,12 @@ qemuGetSEVInfoToParams(virQEMUCapsPtr qemuCaps,
 
     virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
 
+    if (!sev) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("SEV is not supported in this guest"));
+        return -1;
+    }
+
     if (virTypedParamsAddString(&sevParams, &n, &maxpar,
                     VIR_NODE_SEV_PDH, sev->pdh) < 0)
         return -1;
-- 
2.18.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Check sev capability pointer before using it
Posted by Peter Krempa 5 years, 8 months ago
On Tue, Aug 07, 2018 at 09:42:05 +0800, Han Han wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1612009
> 
> Check sev capability pointer in function qemuGetSEVInfoToParams to avoid
> null pointer dereferences.
> 
> Signed-off-by: Han Han <hhan@redhat.com>
> ---
>  src/qemu/qemu_driver.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index fb0d4a8c7a..3daaef586f 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -21452,6 +21452,12 @@ qemuGetSEVInfoToParams(virQEMUCapsPtr qemuCaps,
>  
>      virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
>  
> +    if (!sev) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("SEV is not supported in this guest"));
> +        return -1;
> +    }

I presume the crash happens after restart of libvirtd. The real bug is
that qemuCaps don't serialize the sev data into the status XML thus the
pointer will be cleared and NULL after libvirtd restart.

The error message reported here is then wrong since the guest/host
support SEV but the data is not available.

The crash would not happen otherwise as the function is guarded by
checking QEMU_CAPS_SEV_GUEST.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Check sev capability pointer before using it
Posted by Erik Skultety 5 years, 8 months ago
On Tue, Aug 07, 2018 at 09:05:09AM +0200, Peter Krempa wrote:
> On Tue, Aug 07, 2018 at 09:42:05 +0800, Han Han wrote:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1612009
> >
> > Check sev capability pointer in function qemuGetSEVInfoToParams to avoid
> > null pointer dereferences.
> >
> > Signed-off-by: Han Han <hhan@redhat.com>
> > ---
> >  src/qemu/qemu_driver.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index fb0d4a8c7a..3daaef586f 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -21452,6 +21452,12 @@ qemuGetSEVInfoToParams(virQEMUCapsPtr qemuCaps,
> >
> >      virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
> >
> > +    if (!sev) {
> > +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> > +                       _("SEV is not supported in this guest"));
> > +        return -1;
> > +    }
>
> I presume the crash happens after restart of libvirtd. The real bug is
> that qemuCaps don't serialize the sev data into the status XML thus the
> pointer will be cleared and NULL after libvirtd restart.
>
> The error message reported here is then wrong since the guest/host
> support SEV but the data is not available.
>
> The crash would not happen otherwise as the function is guarded by
> checking QEMU_CAPS_SEV_GUEST.

Exactly, I'm currently working on it, Peter is right, this is not the right
fix.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list