[PATCH] qemu_hotplug: Don't dereference NULL pointer @newb in qemuDomainChangeNet()

Michal Privoznik posted 1 patch 3 years, 2 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/2c8261870fef9afa530cc04772695a2f189fd645.1613411977.git.mprivozn@redhat.com
src/qemu/qemu_hotplug.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] qemu_hotplug: Don't dereference NULL pointer @newb in qemuDomainChangeNet()
Posted by Michal Privoznik 3 years, 2 months ago
In one of my previous commits I've made an attempt to restore the
noqueue qdisc on a TAP corresponding to domain's <interface/> if
QoS is cleared out. The commit consisted of two almost identical
hunks. In both the pointer is dereferenced. But in one of them,
the pointer to new bandwidth can't be NULL while in the other it
can leading to a crash.

Fixes: d53b09235398c1320ed2f1b45b640823171467ed
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1919619
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_hotplug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e7863328db..a66354426d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3900,10 +3900,10 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
 
         /* If the old bandwidth was cleared out, restore qdisc. */
         if (virDomainNetTypeSharesHostView(newdev)) {
-            if (!newb->out || newb->out->average == 0)
+            if (!newb || !newb->out || newb->out->average == 0)
                 qemuDomainInterfaceSetDefaultQDisc(driver, newdev);
         } else {
-            if (!newb->in || newb->in->average == 0)
+            if (!newb || !newb->in || newb->in->average == 0)
                 qemuDomainInterfaceSetDefaultQDisc(driver, newdev);
         }
         needReplaceDevDef = true;
-- 
2.26.2

Re: [PATCH] qemu_hotplug: Don't dereference NULL pointer @newb in qemuDomainChangeNet()
Posted by Daniel Henrique Barboza 3 years, 2 months ago

On 2/15/21 2:59 PM, Michal Privoznik wrote:
> In one of my previous commits I've made an attempt to restore the
> noqueue qdisc on a TAP corresponding to domain's <interface/> if
> QoS is cleared out. The commit consisted of two almost identical
> hunks. In both the pointer is dereferenced. But in one of them,
> the pointer to new bandwidth can't be NULL while in the other it
> can leading to a crash.
> 
> Fixes: d53b09235398c1320ed2f1b45b640823171467ed
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1919619
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---


Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   src/qemu/qemu_hotplug.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index e7863328db..a66354426d 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3900,10 +3900,10 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
>   
>           /* If the old bandwidth was cleared out, restore qdisc. */
>           if (virDomainNetTypeSharesHostView(newdev)) {
> -            if (!newb->out || newb->out->average == 0)
> +            if (!newb || !newb->out || newb->out->average == 0)
>                   qemuDomainInterfaceSetDefaultQDisc(driver, newdev);
>           } else {
> -            if (!newb->in || newb->in->average == 0)
> +            if (!newb || !newb->in || newb->in->average == 0)
>                   qemuDomainInterfaceSetDefaultQDisc(driver, newdev);
>           }
>           needReplaceDevDef = true;
>