On Tue, 29 Oct 2019 at 23:01, Michael S. Tsirkin <m...@redhat.com> wrote:
>
> From: Jens Freimann <jfreim...@redhat.com>
>
> This patch adds support to handle failover device pairs of a virtio-net
> device and a (vfio-)pci device, where the virtio-net acts as the standby
> device and the (vfio-)pci device as the primary.
Hi; Coverity reports some dereference-before-NULL-check errors
in this commit:
> +static bool failover_replug_primary(VirtIONet *n, Error **errp)
> +{
> + HotplugHandler *hotplug_ctrl;
> + PCIDevice *pdev = PCI_DEVICE(n->primary_dev);
> +
> + if (!pdev->partially_hotplugged) {
> + return true;
> + }
> + if (!n->primary_device_opts) {
> + n->primary_device_opts = qemu_opts_from_qdict(
> + qemu_find_opts("device"),
> + n->primary_device_dict, errp);
> + }
> + if (n->primary_device_opts) {
> + if (n->primary_dev) {
Here we check whether n->primary_dev is NULL...
> + n->primary_bus = n->primary_dev->parent_bus;
> + }
> + qdev_set_parent_bus(n->primary_dev, n->primary_bus);
...but qdev_set_parent_bus unconditionally dereferences
its first argument, so it can't be NULL...
> + n->primary_should_be_hidden = false;
> + qemu_opt_set_bool(n->primary_device_opts,
> + "partially_hotplugged", true, errp);
> + hotplug_ctrl = qdev_get_hotplug_handler(n->primary_dev);
> + if (hotplug_ctrl) {
> + hotplug_handler_pre_plug(hotplug_ctrl, n->primary_dev, errp);
> + hotplug_handler_plug(hotplug_ctrl, n->primary_dev, errp);
> + }
> + if (!n->primary_dev) {
...and we do another NULL check here.
Either we don't need the NULL checks, or we need to avoid
calling qdev_set_parent_bus(NULL, ...).
(This is CID 1407224.)
> + error_setg(errp, "virtio_net: couldn't find primary device");
> + }
> + }
> + return *errp != NULL;
> +}
> +static int virtio_net_primary_should_be_hidden(DeviceListener *listener,
> + QemuOpts *device_opts)
> +{
> + VirtIONet *n = container_of(listener, VirtIONet, primary_listener);
> + bool match_found;
> + bool hide;
> +
> + n->primary_device_dict = qemu_opts_to_qdict(device_opts,
> + n->primary_device_dict);
Here we pass device_optns to qemu_opts_to_qdict(), which must
take a non-NULL pointer (it always dereferences it)...
> + if (n->primary_device_dict) {
> + g_free(n->standby_id);
> + n->standby_id = g_strdup(qdict_get_try_str(n->primary_device_dict,
> + "failover_pair_id"));
> + }
> + if (device_opts && g_strcmp0(n->standby_id, n->netclient_name) == 0) {
...but here we check whether device_opts is NULL.
Again, either the check or the call must be wrong.
(This is CID 1407222.)
> + match_found = true;
> + } else {
> + match_found = false;
> + hide = false;
> + g_free(n->standby_id);
> + n->primary_device_dict = NULL;
> + goto out;
> + }
thanks
-- PMM
