All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: vsementsov@virtuozzo.com, Jens Freimann <jfreimann@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>
Subject: [PATCH 02/21] net/virtio: Fix failover error handling crash bugs
Date: Sat, 30 Nov 2019 20:42:21 +0100	[thread overview]
Message-ID: <20191130194240.10517-3-armbru@redhat.com> (raw)
In-Reply-To: <20191130194240.10517-1-armbru@redhat.com>

Functions that take an Error ** parameter to pass an error to the
caller expect the parameter to point to null.
failover_replug_primary() violates this precondition in several
places:

* After qemu_opts_from_qdict() failed, *errp is no longer null.
  Passing it to error_setg() is wrong, and will trip the assertion in
  error_setv().  Messed up in commit 150ab54aa6 "net/virtio: fix
  re-plugging of primary device".  Simply drop the error_setg().

* Passing @errp to qemu_opt_set_bool(), hotplug_handler_pre_plug(),
  and hotplug_handler_plug() is wrong.  If one of the first two fails,
  *errp is no longer null.  Risks tripping the same assertion.
  Moreover, continuing after such errors is unsafe.  Messed up in
  commit 9711cd0dfc "net/virtio: add failover support".  Fix by
  handling each error properly.

failover_replug_primary() crashes when passed a null @errp.  Also
messed up in commit 9711cd0dfc.  This bug can't bite as no caller
actually passes null.  Fix it anyway.

Fixes: 9711cd0dfc3fa414f7f64935713c07134ae67971
Fixes: 150ab54aa6934583180f88a2bd540bc6fc4fbff3
Cc: Jens Freimann <jfreimann@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/net/virtio-net.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 87088ba374..db3d7c38e6 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2795,6 +2795,7 @@ static bool failover_unplug_primary(VirtIONet *n)
 
 static bool failover_replug_primary(VirtIONet *n, Error **errp)
 {
+    Error *err = NULL;
     HotplugHandler *hotplug_ctrl;
     PCIDevice *pdev = PCI_DEVICE(n->primary_dev);
 
@@ -2806,27 +2807,33 @@ static bool failover_replug_primary(VirtIONet *n, Error **errp)
                 qemu_find_opts("device"),
                 n->primary_device_dict, errp);
         if (!n->primary_device_opts) {
-            error_setg(errp, "virtio_net: couldn't find primary device opts");
-            goto out;
+            return false;
         }
     }
     n->primary_bus = n->primary_dev->parent_bus;
     if (!n->primary_bus) {
         error_setg(errp, "virtio_net: couldn't find primary bus");
-        goto out;
+        return false;
     }
     qdev_set_parent_bus(n->primary_dev, n->primary_bus);
     n->primary_should_be_hidden = false;
     qemu_opt_set_bool(n->primary_device_opts,
-                      "partially_hotplugged", true, errp);
+                      "partially_hotplugged", true, &err);
+    if (err) {
+        goto out;
+    }
     hotplug_ctrl = qdev_get_hotplug_handler(n->primary_dev);
     if (hotplug_ctrl) {
-        hotplug_handler_pre_plug(hotplug_ctrl, n->primary_dev, errp);
+        hotplug_handler_pre_plug(hotplug_ctrl, n->primary_dev, &err);
+        if (err) {
+            goto out;
+        }
         hotplug_handler_plug(hotplug_ctrl, n->primary_dev, errp);
     }
 
 out:
-    return *errp == NULL;
+    error_propagate(errp, err);
+    return !err;
 }
 
 static void virtio_net_handle_migration_primary(VirtIONet *n,
-- 
2.21.0



  parent reply	other threads:[~2019-11-30 19:45 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-30 19:42 [PATCH 00/21] Error handling fixes, may contain 4.2 material Markus Armbruster
2019-11-30 19:42 ` [PATCH 01/21] net/virtio: Drop useless n->primary_dev not null checks Markus Armbruster
2019-12-02  9:53   ` Jens Freimann
2019-11-30 19:42 ` Markus Armbruster [this message]
2019-12-02  9:53   ` [PATCH 02/21] net/virtio: Fix failover error handling crash bugs Jens Freimann
2019-11-30 19:42 ` [PATCH 03/21] block/file-posix: Fix laio_init() error handling crash bug Markus Armbruster
2019-12-02 10:04   ` Stefan Hajnoczi
2019-12-02 12:22   ` Kevin Wolf
2019-11-30 19:42 ` [PATCH 04/21] crypto: Fix certificate file " Markus Armbruster
2019-12-05 15:24   ` Vladimir Sementsov-Ogievskiy
2019-11-30 19:42 ` [PATCH 05/21] crypto: Fix typo in QCryptoTLSSession's <example> comment Markus Armbruster
2019-12-05 15:26   ` Vladimir Sementsov-Ogievskiy
2019-11-30 19:42 ` [PATCH 06/21] io: Fix Error usage in a comment <example> Markus Armbruster
2019-12-05 15:30   ` Vladimir Sementsov-Ogievskiy
2019-12-06  7:20     ` Markus Armbruster
2019-11-30 19:42 ` [PATCH 07/21] tests: Clean up initialization of Error *err variables Markus Armbruster
2019-12-05 15:33   ` Vladimir Sementsov-Ogievskiy
2019-11-30 19:42 ` [PATCH 08/21] exec: Fix latent file_ram_alloc() error handling bug Markus Armbruster
2019-12-02  7:46   ` Igor Mammedov
2019-11-30 19:42 ` [PATCH 09/21] hw/acpi: Fix latent legacy CPU plug " Markus Armbruster
2019-12-02  7:51   ` Igor Mammedov
2019-11-30 19:42 ` [PATCH 10/21] hw/core: Fix latent fit_load_fdt() " Markus Armbruster
2019-12-05 16:23   ` Vladimir Sementsov-Ogievskiy
2019-12-06  7:46     ` Markus Armbruster
2019-12-06 10:53       ` Vladimir Sementsov-Ogievskiy
2020-01-10 20:06         ` Vladimir Sementsov-Ogievskiy
2020-01-13 13:01           ` Markus Armbruster
2019-11-30 19:42 ` [PATCH 11/21] hw/ipmi: Fix latent realize() error handling bugs Markus Armbruster
2019-12-01 18:22   ` Corey Minyard
2019-12-16  9:20     ` Markus Armbruster
2019-12-16 14:13       ` Corey Minyard
2019-12-17  6:30         ` Markus Armbruster
2019-11-30 19:42 ` [PATCH 12/21] qga: Fix latent guest-get-fsinfo error handling bug Markus Armbruster
2019-12-05 16:29   ` Vladimir Sementsov-Ogievskiy
2019-12-06  7:58     ` Markus Armbruster
2019-11-30 19:42 ` [PATCH 13/21] memory-device: Fix latent memory pre-plug error handling bugs Markus Armbruster
2019-12-01 14:15   ` David Hildenbrand
2019-12-02  5:07     ` Markus Armbruster
2019-12-03 21:37       ` Eric Blake
2019-11-30 19:42 ` [PATCH 14/21] s390x/event-facility: Fix latent realize() error handling bug Markus Armbruster
2019-12-02  9:56   ` David Hildenbrand
2019-11-30 19:42 ` [PATCH 15/21] s390x/cpu_models: Fix latent feature property error handling bugs Markus Armbruster
2019-12-02  9:57   ` David Hildenbrand
2019-12-03  7:22     ` Markus Armbruster
2019-11-30 19:42 ` [PATCH 16/21] s390/cpu_modules: Fix latent realize() " Markus Armbruster
2019-12-01 14:25   ` David Hildenbrand
2019-12-02  5:02     ` Markus Armbruster
2019-11-30 19:42 ` [PATCH 17/21] s390x: Fix latent query-cpu-model-FOO " Markus Armbruster
2019-11-30 21:22   ` David Hildenbrand
2019-12-01 13:46     ` Aleksandar Markovic
2019-12-01 14:07       ` Aleksandar Markovic
2019-12-01 14:11         ` Aleksandar Markovic
2019-12-01 14:09       ` David Hildenbrand
2019-12-02  5:01         ` Markus Armbruster
2019-12-02  8:34           ` David Hildenbrand
2019-12-03  7:27             ` Markus Armbruster
2019-12-02 16:31   ` Cornelia Huck
2019-12-03  7:49     ` Markus Armbruster
2019-12-03  8:01       ` Cornelia Huck
2019-12-03  9:51         ` David Hildenbrand
2019-11-30 19:42 ` [PATCH 18/21] s390x: Fix latent query-cpu-definitions error handling bug Markus Armbruster
2019-12-02  9:55   ` David Hildenbrand
2019-11-30 19:42 ` [PATCH 19/21] error: Clean up unusual names of Error * variables Markus Armbruster
2019-11-30 20:03   ` Philippe Mathieu-Daudé
2019-11-30 19:42 ` [PATCH 20/21] hw/intc/s390: Simplify error handling in kvm_s390_flic_realize() Markus Armbruster
2019-12-02 16:40   ` Cornelia Huck
2019-12-03 20:03     ` Halil Pasic
2019-12-04  7:28       ` Markus Armbruster
2019-11-30 19:42 ` [PATCH 21/21] tests-blockjob: Use error_free_or_abort() Markus Armbruster
2019-12-03 21:43   ` Eric Blake
2019-12-01 14:44 ` [PATCH 00/21] Error handling fixes, may contain 4.2 material Michael S. Tsirkin
2019-12-04  8:44   ` Markus Armbruster
2019-12-02 10:19 ` Daniel P. Berrangé
2019-12-02 11:24 ` Jens Freimann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191130194240.10517-3-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=jfreimann@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.