[PATCH] hw/pci: Fix crash when running QEMU with "-nic model=rocker"

[PATCH] hw/pci: Fix crash when running QEMU with "-nic model=rocker"

[Thomas Huth]

QEMU currently aborts when being started with "-nic model=rocker" or with
"-net nic,model=rocker". This happens because the "rocker" device is not
a normal NIC but a switch, which has different properties. Thus we should
only consider real NIC devices for "-nic" and "-net". These devices can
be identified by the "netdev" property, so check for this property before
adding the device to the list.

Reported-by: Michael Tokarev <mjt@tls.msk.ru>
Fixes: 52310c3fa7dc854d ("net: allow using any PCI NICs in -net or -nic")
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/pci/pci.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 70c66965f5..46214f8287 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1887,7 +1887,18 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
         if (test_bit(DEVICE_CATEGORY_NETWORK, dc->categories) &&
             dc->user_creatable) {
             const char *name = object_class_get_name(list->data);
-            g_ptr_array_add(pci_nic_models, (gpointer)name);
+            /*
+             * A network device might also be something else than a NIC, see
+             * e.g. the "rocker" device. Thus we have to look for the "netdev"
+             * property, too. Unfortunately, some devices like virtio-net only
+             * create this property during instance_init, so we have to create
+             * a temporary instance here to be able to check it.
+             */
+            Object *obj = object_new_with_class(OBJECT_CLASS(dc));
+            if (object_property_find(obj, "netdev", NULL)) {
+                g_ptr_array_add(pci_nic_models, (gpointer)name);
+            }
+            object_unref(obj);
         }
         next = list->next;
         g_slist_free_1(list);
-- 
2.18.1

Re: [PATCH] hw/pci: Fix crash when running QEMU with "-nic model=rocker"

[Paolo Bonzini]

On 27/05/20 17:31, Thomas Huth wrote:
> QEMU currently aborts when being started with "-nic model=rocker" or with
> "-net nic,model=rocker". This happens because the "rocker" device is not
> a normal NIC but a switch, which has different properties. Thus we should
> only consider real NIC devices for "-nic" and "-net". These devices can
> be identified by the "netdev" property, so check for this property before
> adding the device to the list.
> 
> Reported-by: Michael Tokarev <mjt@tls.msk.ru>
> Fixes: 52310c3fa7dc854d ("net: allow using any PCI NICs in -net or -nic")
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/pci/pci.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 70c66965f5..46214f8287 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1887,7 +1887,18 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
>          if (test_bit(DEVICE_CATEGORY_NETWORK, dc->categories) &&
>              dc->user_creatable) {
>              const char *name = object_class_get_name(list->data);
> -            g_ptr_array_add(pci_nic_models, (gpointer)name);
> +            /*
> +             * A network device might also be something else than a NIC, see
> +             * e.g. the "rocker" device. Thus we have to look for the "netdev"
> +             * property, too. Unfortunately, some devices like virtio-net only
> +             * create this property during instance_init, so we have to create
> +             * a temporary instance here to be able to check it.
> +             */
> +            Object *obj = object_new_with_class(OBJECT_CLASS(dc));
> +            if (object_property_find(obj, "netdev", NULL)) {
> +                g_ptr_array_add(pci_nic_models, (gpointer)name);
> +            }
> +            object_unref(obj);
>          }
>          next = list->next;
>          g_slist_free_1(list);
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [PATCH] hw/pci: Fix crash when running QEMU with "-nic model=rocker"

[Laurent Vivier]

Le 27/05/2020 à 17:31, Thomas Huth a écrit :
> QEMU currently aborts when being started with "-nic model=rocker" or with
> "-net nic,model=rocker". This happens because the "rocker" device is not
> a normal NIC but a switch, which has different properties. Thus we should
> only consider real NIC devices for "-nic" and "-net". These devices can
> be identified by the "netdev" property, so check for this property before
> adding the device to the list.
> 
> Reported-by: Michael Tokarev <mjt@tls.msk.ru>
> Fixes: 52310c3fa7dc854d ("net: allow using any PCI NICs in -net or -nic")
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/pci/pci.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 70c66965f5..46214f8287 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1887,7 +1887,18 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
>          if (test_bit(DEVICE_CATEGORY_NETWORK, dc->categories) &&
>              dc->user_creatable) {
>              const char *name = object_class_get_name(list->data);
> -            g_ptr_array_add(pci_nic_models, (gpointer)name);
> +            /*
> +             * A network device might also be something else than a NIC, see
> +             * e.g. the "rocker" device. Thus we have to look for the "netdev"
> +             * property, too. Unfortunately, some devices like virtio-net only
> +             * create this property during instance_init, so we have to create
> +             * a temporary instance here to be able to check it.
> +             */
> +            Object *obj = object_new_with_class(OBJECT_CLASS(dc));
> +            if (object_property_find(obj, "netdev", NULL)) {
> +                g_ptr_array_add(pci_nic_models, (gpointer)name);
> +            }
> +            object_unref(obj);
>          }
>          next = list->next;
>          g_slist_free_1(list);
> 

Not really trivial, I will not pick up this patch via trivial-branch,
unless PCI maintainers request it.

Thanks,
Laurent

Re: [PATCH] hw/pci: Fix crash when running QEMU with "-nic model=rocker"

[Paolo Bonzini]

On 09/06/20 19:21, Laurent Vivier wrote:
> Le 27/05/2020 à 17:31, Thomas Huth a écrit :
>> QEMU currently aborts when being started with "-nic model=rocker" or with
>> "-net nic,model=rocker". This happens because the "rocker" device is not
>> a normal NIC but a switch, which has different properties. Thus we should
>> only consider real NIC devices for "-nic" and "-net". These devices can
>> be identified by the "netdev" property, so check for this property before
>> adding the device to the list.
>>
>> Reported-by: Michael Tokarev <mjt@tls.msk.ru>
>> Fixes: 52310c3fa7dc854d ("net: allow using any PCI NICs in -net or -nic")
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  hw/pci/pci.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>> index 70c66965f5..46214f8287 100644
>> --- a/hw/pci/pci.c
>> +++ b/hw/pci/pci.c
>> @@ -1887,7 +1887,18 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
>>          if (test_bit(DEVICE_CATEGORY_NETWORK, dc->categories) &&
>>              dc->user_creatable) {
>>              const char *name = object_class_get_name(list->data);
>> -            g_ptr_array_add(pci_nic_models, (gpointer)name);
>> +            /*
>> +             * A network device might also be something else than a NIC, see
>> +             * e.g. the "rocker" device. Thus we have to look for the "netdev"
>> +             * property, too. Unfortunately, some devices like virtio-net only
>> +             * create this property during instance_init, so we have to create
>> +             * a temporary instance here to be able to check it.
>> +             */
>> +            Object *obj = object_new_with_class(OBJECT_CLASS(dc));
>> +            if (object_property_find(obj, "netdev", NULL)) {
>> +                g_ptr_array_add(pci_nic_models, (gpointer)name);
>> +            }
>> +            object_unref(obj);
>>          }
>>          next = list->next;
>>          g_slist_free_1(list);
>>
> 
> Not really trivial, I will not pick up this patch via trivial-branch,
> unless PCI maintainers request it.
> 
> Thanks,
> Laurent
> 

I'll pick it up then.

Paolo

Re: [PATCH] hw/pci: Fix crash when running QEMU with "-nic model=rocker"

[Michael S. Tsirkin]

On Tue, Jun 09, 2020 at 07:24:24PM +0200, Paolo Bonzini wrote:
> On 09/06/20 19:21, Laurent Vivier wrote:
> > Le 27/05/2020 à 17:31, Thomas Huth a écrit :
> >> QEMU currently aborts when being started with "-nic model=rocker" or with
> >> "-net nic,model=rocker". This happens because the "rocker" device is not
> >> a normal NIC but a switch, which has different properties. Thus we should
> >> only consider real NIC devices for "-nic" and "-net". These devices can
> >> be identified by the "netdev" property, so check for this property before
> >> adding the device to the list.
> >>
> >> Reported-by: Michael Tokarev <mjt@tls.msk.ru>
> >> Fixes: 52310c3fa7dc854d ("net: allow using any PCI NICs in -net or -nic")
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >> ---
> >>  hw/pci/pci.c | 13 ++++++++++++-
> >>  1 file changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> >> index 70c66965f5..46214f8287 100644
> >> --- a/hw/pci/pci.c
> >> +++ b/hw/pci/pci.c
> >> @@ -1887,7 +1887,18 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
> >>          if (test_bit(DEVICE_CATEGORY_NETWORK, dc->categories) &&
> >>              dc->user_creatable) {
> >>              const char *name = object_class_get_name(list->data);
> >> -            g_ptr_array_add(pci_nic_models, (gpointer)name);
> >> +            /*
> >> +             * A network device might also be something else than a NIC, see
> >> +             * e.g. the "rocker" device. Thus we have to look for the "netdev"
> >> +             * property, too. Unfortunately, some devices like virtio-net only
> >> +             * create this property during instance_init, so we have to create
> >> +             * a temporary instance here to be able to check it.
> >> +             */
> >> +            Object *obj = object_new_with_class(OBJECT_CLASS(dc));
> >> +            if (object_property_find(obj, "netdev", NULL)) {
> >> +                g_ptr_array_add(pci_nic_models, (gpointer)name);
> >> +            }
> >> +            object_unref(obj);
> >>          }
> >>          next = list->next;
> >>          g_slist_free_1(list);
> >>
> > 
> > Not really trivial, I will not pick up this patch via trivial-branch,
> > unless PCI maintainers request it.
> > 
> > Thanks,
> > Laurent
> > 
> 
> I'll pick it up then.
> 
> Paolo

I queued this already. Thanks!

-- 
MST