[Qemu-devel] [PATCH] Fix crash with illegal "-net nic, model=xxx" option

[Qemu-devel] [PATCH] Fix crash with illegal "-net nic, model=xxx" option

[Thomas Huth]

Current QEMU crashes when specifying an illegal model with the
"-net nic,model=xxx" option, e.g.:

 $ qemu-system-x86_64 -net nic,model=n/a
 qemu-system-x86_64: Unsupported NIC model: n/a

 Program received signal SIGSEGV, Segmentation fault.

The gdb backtrace looks like this:

0x0000555555965fe0 in error_get_pretty (err=0x0) at util/error.c:152
152	    return err->msg;
(gdb) bt
 0  0x0000555555965fe0 in error_get_pretty (err=0x0) at util/error.c:152
 1  0x0000555555965ffd in error_report_err (err=0x0) at util/error.c:157
 2  0x0000555555809c90 in pci_nic_init_nofail (nd=0x555555e49860 <nd_table>, rootbus=0x5555564409b0,
    default_model=0x55555598c37b "e1000", default_devaddr=0x0) at hw/pci/pci.c:1663
 3  0x0000555555691e42 in pc_nic_init (isa_bus=0x555556f71900, pci_bus=0x5555564409b0)
    at hw/i386/pc.c:1506
 4  0x000055555569396b in pc_init1 (machine=0x5555562abbf0, pci_enabled=1, kvmclock_enabled=1)
    at hw/i386/pc_piix.c:248
 5  0x0000555555693d27 in pc_init_pci (machine=0x5555562abbf0) at hw/i386/pc_piix.c:310
 6  0x000055555572ddf5 in main (argc=3, argv=0x7fffffffe018, envp=0x7fffffffe038) at vl.c:4226

The problem is that pci_nic_init_nofail() does not check whether the err
parameter from pci_nic_init has been set up and thus passes a NULL pointer
to error_report_err(). Fix it by correctly checking the err parameter.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/pci/pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 6941a82..b3d5100 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1660,7 +1660,9 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
 
     res = pci_nic_init(nd, rootbus, default_model, default_devaddr, &err);
     if (!res) {
-        error_report_err(err);
+        if (err) {
+            error_report_err(err);
+        }
         exit(1);
     }
     return res;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] Fix crash with illegal "-net nic, model=xxx" option

[Michael S. Tsirkin]

On Thu, Apr 09, 2015 at 03:32:45PM +0200, Thomas Huth wrote:
> Current QEMU crashes when specifying an illegal model with the
> "-net nic,model=xxx" option, e.g.:
> 
>  $ qemu-system-x86_64 -net nic,model=n/a
>  qemu-system-x86_64: Unsupported NIC model: n/a
> 
>  Program received signal SIGSEGV, Segmentation fault.
> 
> The gdb backtrace looks like this:
> 
> 0x0000555555965fe0 in error_get_pretty (err=0x0) at util/error.c:152
> 152	    return err->msg;
> (gdb) bt
>  0  0x0000555555965fe0 in error_get_pretty (err=0x0) at util/error.c:152
>  1  0x0000555555965ffd in error_report_err (err=0x0) at util/error.c:157
>  2  0x0000555555809c90 in pci_nic_init_nofail (nd=0x555555e49860 <nd_table>, rootbus=0x5555564409b0,
>     default_model=0x55555598c37b "e1000", default_devaddr=0x0) at hw/pci/pci.c:1663
>  3  0x0000555555691e42 in pc_nic_init (isa_bus=0x555556f71900, pci_bus=0x5555564409b0)
>     at hw/i386/pc.c:1506
>  4  0x000055555569396b in pc_init1 (machine=0x5555562abbf0, pci_enabled=1, kvmclock_enabled=1)
>     at hw/i386/pc_piix.c:248
>  5  0x0000555555693d27 in pc_init_pci (machine=0x5555562abbf0) at hw/i386/pc_piix.c:310
>  6  0x000055555572ddf5 in main (argc=3, argv=0x7fffffffe018, envp=0x7fffffffe038) at vl.c:4226
> 
> The problem is that pci_nic_init_nofail() does not check whether the err
> parameter from pci_nic_init has been set up and thus passes a NULL pointer
> to error_report_err(). Fix it by correctly checking the err parameter.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Thanks!
Given that this is a legacy -net option, I'm inclined
to fix it post-2.3, and Cc stable.
Unfortunately I won't be able to do a pull request before rc3.

> ---
>  hw/pci/pci.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 6941a82..b3d5100 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1660,7 +1660,9 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
>  
>      res = pci_nic_init(nd, rootbus, default_model, default_devaddr, &err);
>      if (!res) {
> -        error_report_err(err);
> +        if (err) {
> +            error_report_err(err);
> +        }
>          exit(1);
>      }
>      return res;
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH] Fix crash with illegal "-net nic, model=xxx" option

[Peter Maydell]

On 9 April 2015 at 14:37, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Apr 09, 2015 at 03:32:45PM +0200, Thomas Huth wrote:
>> Current QEMU crashes when specifying an illegal model with the
>> "-net nic,model=xxx" option, e.g.:
>>
>>  $ qemu-system-x86_64 -net nic,model=n/a
>>  qemu-system-x86_64: Unsupported NIC model: n/a
>>
>>  Program received signal SIGSEGV, Segmentation fault.
>>
>> The gdb backtrace looks like this:
>>
>> 0x0000555555965fe0 in error_get_pretty (err=0x0) at util/error.c:152
>> 152       return err->msg;
>> (gdb) bt
>>  0  0x0000555555965fe0 in error_get_pretty (err=0x0) at util/error.c:152
>>  1  0x0000555555965ffd in error_report_err (err=0x0) at util/error.c:157
>>  2  0x0000555555809c90 in pci_nic_init_nofail (nd=0x555555e49860 <nd_table>, rootbus=0x5555564409b0,
>>     default_model=0x55555598c37b "e1000", default_devaddr=0x0) at hw/pci/pci.c:1663
>>  3  0x0000555555691e42 in pc_nic_init (isa_bus=0x555556f71900, pci_bus=0x5555564409b0)
>>     at hw/i386/pc.c:1506
>>  4  0x000055555569396b in pc_init1 (machine=0x5555562abbf0, pci_enabled=1, kvmclock_enabled=1)
>>     at hw/i386/pc_piix.c:248
>>  5  0x0000555555693d27 in pc_init_pci (machine=0x5555562abbf0) at hw/i386/pc_piix.c:310
>>  6  0x000055555572ddf5 in main (argc=3, argv=0x7fffffffe018, envp=0x7fffffffe038) at vl.c:4226
>>
>> The problem is that pci_nic_init_nofail() does not check whether the err
>> parameter from pci_nic_init has been set up and thus passes a NULL pointer
>> to error_report_err(). Fix it by correctly checking the err parameter.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>
> Thanks!
> Given that this is a legacy -net option, I'm inclined
> to fix it post-2.3, and Cc stable.
> Unfortunately I won't be able to do a pull request before rc3.

Since this is a pretty safe and simple change I'm happy to apply
it direct to master if you like. Do you want to provide a reviewed-by
tag?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] Fix crash with illegal "-net nic, model=xxx" option

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1373 bytes --]

On 04/09/2015 07:32 AM, Thomas Huth wrote:
> Current QEMU crashes when specifying an illegal model with the
> "-net nic,model=xxx" option, e.g.:
> 
>  $ qemu-system-x86_64 -net nic,model=n/a
>  qemu-system-x86_64: Unsupported NIC model: n/a
> 

> The problem is that pci_nic_init_nofail() does not check whether the err
> parameter from pci_nic_init has been set up and thus passes a NULL pointer
> to error_report_err(). Fix it by correctly checking the err parameter.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/pci/pci.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 6941a82..b3d5100 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1660,7 +1660,9 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
>  
>      res = pci_nic_init(nd, rootbus, default_model, default_devaddr, &err);
>      if (!res) {
> -        error_report_err(err);
> +        if (err) {
> +            error_report_err(err);
> +        }
>          exit(1);

Doesn't this mean the program can exit without an error message, if
pci_nic_init returns failure but failed to set err?  Shouldn't you at
least print something in that case as an else branch?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] Fix crash with illegal "-net nic, model=xxx" option

[Paolo Bonzini]

On 09/04/2015 20:31, Eric Blake wrote:
>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>> index 6941a82..b3d5100 100644
>> --- a/hw/pci/pci.c
>> +++ b/hw/pci/pci.c
>> @@ -1660,7 +1660,9 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
>>  
>>      res = pci_nic_init(nd, rootbus, default_model, default_devaddr, &err);
>>      if (!res) {
>> -        error_report_err(err);
>> +        if (err) {
>> +            error_report_err(err);
>> +        }
>>          exit(1);
> 
> Doesn't this mean the program can exit without an error message, if
> pci_nic_init returns failure but failed to set err?  Shouldn't you at
> least print something in that case as an else branch?

git grep 'Unsupported NIC model' shows that the error is printed with
error_report; same for other errors produced by pci_nic_init.

This is not beautiful compared to correct propagation of Error*, but
it's okay because -net is only used at startup.  It's good enough for rc3.

Paolo

Re: [Qemu-devel] [PATCH] Fix crash with illegal "-net nic, model=xxx" option

[Michael S. Tsirkin]

On Thu, Apr 09, 2015 at 03:32:45PM +0200, Thomas Huth wrote:
> Current QEMU crashes when specifying an illegal model with the
> "-net nic,model=xxx" option, e.g.:
> 
>  $ qemu-system-x86_64 -net nic,model=n/a
>  qemu-system-x86_64: Unsupported NIC model: n/a
> 
>  Program received signal SIGSEGV, Segmentation fault.
> 
> The gdb backtrace looks like this:
> 
> 0x0000555555965fe0 in error_get_pretty (err=0x0) at util/error.c:152
> 152	    return err->msg;
> (gdb) bt
>  0  0x0000555555965fe0 in error_get_pretty (err=0x0) at util/error.c:152
>  1  0x0000555555965ffd in error_report_err (err=0x0) at util/error.c:157
>  2  0x0000555555809c90 in pci_nic_init_nofail (nd=0x555555e49860 <nd_table>, rootbus=0x5555564409b0,
>     default_model=0x55555598c37b "e1000", default_devaddr=0x0) at hw/pci/pci.c:1663
>  3  0x0000555555691e42 in pc_nic_init (isa_bus=0x555556f71900, pci_bus=0x5555564409b0)
>     at hw/i386/pc.c:1506
>  4  0x000055555569396b in pc_init1 (machine=0x5555562abbf0, pci_enabled=1, kvmclock_enabled=1)
>     at hw/i386/pc_piix.c:248
>  5  0x0000555555693d27 in pc_init_pci (machine=0x5555562abbf0) at hw/i386/pc_piix.c:310
>  6  0x000055555572ddf5 in main (argc=3, argv=0x7fffffffe018, envp=0x7fffffffe038) at vl.c:4226
> 
> The problem is that pci_nic_init_nofail() does not check whether the err
> parameter from pci_nic_init has been set up and thus passes a NULL pointer
> to error_report_err(). Fix it by correctly checking the err parameter.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/pci/pci.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 6941a82..b3d5100 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1660,7 +1660,9 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
>  
>      res = pci_nic_init(nd, rootbus, default_model, default_devaddr, &err);
>      if (!res) {
> -        error_report_err(err);
> +        if (err) {
> +            error_report_err(err);
> +        }
>          exit(1);
>      }
>      return res;
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH] Fix crash with illegal "-net nic, model=xxx" option

[Michael S. Tsirkin]

On Thu, Apr 09, 2015 at 03:48:57PM +0100, Peter Maydell wrote:
> On 9 April 2015 at 14:37, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Thu, Apr 09, 2015 at 03:32:45PM +0200, Thomas Huth wrote:
> >> Current QEMU crashes when specifying an illegal model with the
> >> "-net nic,model=xxx" option, e.g.:
> >>
> >>  $ qemu-system-x86_64 -net nic,model=n/a
> >>  qemu-system-x86_64: Unsupported NIC model: n/a
> >>
> >>  Program received signal SIGSEGV, Segmentation fault.
> >>
> >> The gdb backtrace looks like this:
> >>
> >> 0x0000555555965fe0 in error_get_pretty (err=0x0) at util/error.c:152
> >> 152       return err->msg;
> >> (gdb) bt
> >>  0  0x0000555555965fe0 in error_get_pretty (err=0x0) at util/error.c:152
> >>  1  0x0000555555965ffd in error_report_err (err=0x0) at util/error.c:157
> >>  2  0x0000555555809c90 in pci_nic_init_nofail (nd=0x555555e49860 <nd_table>, rootbus=0x5555564409b0,
> >>     default_model=0x55555598c37b "e1000", default_devaddr=0x0) at hw/pci/pci.c:1663
> >>  3  0x0000555555691e42 in pc_nic_init (isa_bus=0x555556f71900, pci_bus=0x5555564409b0)
> >>     at hw/i386/pc.c:1506
> >>  4  0x000055555569396b in pc_init1 (machine=0x5555562abbf0, pci_enabled=1, kvmclock_enabled=1)
> >>     at hw/i386/pc_piix.c:248
> >>  5  0x0000555555693d27 in pc_init_pci (machine=0x5555562abbf0) at hw/i386/pc_piix.c:310
> >>  6  0x000055555572ddf5 in main (argc=3, argv=0x7fffffffe018, envp=0x7fffffffe038) at vl.c:4226
> >>
> >> The problem is that pci_nic_init_nofail() does not check whether the err
> >> parameter from pci_nic_init has been set up and thus passes a NULL pointer
> >> to error_report_err(). Fix it by correctly checking the err parameter.
> >>
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >
> > Thanks!
> > Given that this is a legacy -net option, I'm inclined
> > to fix it post-2.3, and Cc stable.
> > Unfortunately I won't be able to do a pull request before rc3.
> 
> Since this is a pretty safe and simple change I'm happy to apply
> it direct to master if you like. Do you want to provide a reviewed-by
> tag?
> 
> thanks
> -- PMM

I reviewed the patch, and sent a tag.
I'm fine with you making the decision on whether it's
appropriate for 2.3.

-- 
MST

Re: [Qemu-devel] [PATCH] Fix crash with illegal "-net nic, model=xxx" option

[Andreas Färber]

Am 12.04.2015 um 13:14 schrieb Michael S. Tsirkin:
> On Thu, Apr 09, 2015 at 03:48:57PM +0100, Peter Maydell wrote:
>> On 9 April 2015 at 14:37, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> On Thu, Apr 09, 2015 at 03:32:45PM +0200, Thomas Huth wrote:
>>>> Current QEMU crashes when specifying an illegal model with the
>>>> "-net nic,model=xxx" option, e.g.:
>>>>
>>>>  $ qemu-system-x86_64 -net nic,model=n/a
>>>>  qemu-system-x86_64: Unsupported NIC model: n/a
>>>>
>>>>  Program received signal SIGSEGV, Segmentation fault.
>>>>
>>>> The gdb backtrace looks like this:
>>>>
>>>> 0x0000555555965fe0 in error_get_pretty (err=0x0) at util/error.c:152
>>>> 152       return err->msg;
>>>> (gdb) bt
>>>>  0  0x0000555555965fe0 in error_get_pretty (err=0x0) at util/error.c:152
>>>>  1  0x0000555555965ffd in error_report_err (err=0x0) at util/error.c:157
>>>>  2  0x0000555555809c90 in pci_nic_init_nofail (nd=0x555555e49860 <nd_table>, rootbus=0x5555564409b0,
>>>>     default_model=0x55555598c37b "e1000", default_devaddr=0x0) at hw/pci/pci.c:1663
>>>>  3  0x0000555555691e42 in pc_nic_init (isa_bus=0x555556f71900, pci_bus=0x5555564409b0)
>>>>     at hw/i386/pc.c:1506
>>>>  4  0x000055555569396b in pc_init1 (machine=0x5555562abbf0, pci_enabled=1, kvmclock_enabled=1)
>>>>     at hw/i386/pc_piix.c:248
>>>>  5  0x0000555555693d27 in pc_init_pci (machine=0x5555562abbf0) at hw/i386/pc_piix.c:310
>>>>  6  0x000055555572ddf5 in main (argc=3, argv=0x7fffffffe018, envp=0x7fffffffe038) at vl.c:4226
>>>>
>>>> The problem is that pci_nic_init_nofail() does not check whether the err
>>>> parameter from pci_nic_init has been set up and thus passes a NULL pointer
>>>> to error_report_err(). Fix it by correctly checking the err parameter.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>
>>> Thanks!
>>> Given that this is a legacy -net option, I'm inclined
>>> to fix it post-2.3, and Cc stable.
>>> Unfortunately I won't be able to do a pull request before rc3.
>>
>> Since this is a pretty safe and simple change I'm happy to apply
>> it direct to master if you like. Do you want to provide a reviewed-by
>> tag?
> 
> I reviewed the patch, and sent a tag.
> I'm fine with you making the decision on whether it's
> appropriate for 2.3.

Hope you added a "pci:" topic? :)

Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH] Fix crash with illegal "-net nic, model=xxx" option

[Jason Wang]

On Thu, Apr 9, 2015 at 9:32 PM, Thomas Huth <thuth@redhat.com> wrote:
> Current QEMU crashes when specifying an illegal model with the
> "-net nic,model=xxx" option, e.g.:
> 
>  $ qemu-system-x86_64 -net nic,model=n/a
>  qemu-system-x86_64: Unsupported NIC model: n/a
> 
>  Program received signal SIGSEGV, Segmentation fault.
> 
> The gdb backtrace looks like this:
> 
> 0x0000555555965fe0 in error_get_pretty (err=0x0) at util/error.c:152
> 152	    return err->msg;
> (gdb) bt
>  0  0x0000555555965fe0 in error_get_pretty (err=0x0) at 
> util/error.c:152
>  1  0x0000555555965ffd in error_report_err (err=0x0) at 
> util/error.c:157
>  2  0x0000555555809c90 in pci_nic_init_nofail (nd=0x555555e49860 
> <nd_table>, rootbus=0x5555564409b0,
>     default_model=0x55555598c37b "e1000", default_devaddr=0x0) at 
> hw/pci/pci.c:1663
>  3  0x0000555555691e42 in pc_nic_init (isa_bus=0x555556f71900, 
> pci_bus=0x5555564409b0)
>     at hw/i386/pc.c:1506
>  4  0x000055555569396b in pc_init1 (machine=0x5555562abbf0, 
> pci_enabled=1, kvmclock_enabled=1)
>     at hw/i386/pc_piix.c:248
>  5  0x0000555555693d27 in pc_init_pci (machine=0x5555562abbf0) at 
> hw/i386/pc_piix.c:310
>  6  0x000055555572ddf5 in main (argc=3, argv=0x7fffffffe018, 
> envp=0x7fffffffe038) at vl.c:4226
> 
> The problem is that pci_nic_init_nofail() does not check whether the 
> err
> parameter from pci_nic_init has been set up and thus passes a NULL 
> pointer
> to error_report_err(). Fix it by correctly checking the err parameter.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/pci/pci.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 6941a82..b3d5100 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1660,7 +1660,9 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, 
> PCIBus *rootbus,
>  
>      res = pci_nic_init(nd, rootbus, default_model, default_devaddr, 
> &err);
>      if (!res) {
> -        error_report_err(err);
> +        if (err) {
> +            error_report_err(err);
> +        }
>          exit(1);
>      }
>      return res;
> -- 
> 1.8.3.1

Reviewed-by: Jason Wang <jasowang@redhat.com>

Re: [Qemu-devel] [PATCH] Fix crash with illegal "-net nic, model=xxx" option

[Peter Maydell]

On 13 April 2015 at 04:01, Jason Wang <jasowang@redhat.com> wrote:
>
>
> On Thu, Apr 9, 2015 at 9:32 PM, Thomas Huth <thuth@redhat.com> wrote:
>>
>> Current QEMU crashes when specifying an illegal model with the
>> "-net nic,model=xxx" option, e.g.:
>>
>>  $ qemu-system-x86_64 -net nic,model=n/a
>>  qemu-system-x86_64: Unsupported NIC model: n/a

>> The problem is that pci_nic_init_nofail() does not check whether the err
>> parameter from pci_nic_init has been set up and thus passes a NULL pointer
>> to error_report_err(). Fix it by correctly checking the err parameter.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>

> Reviewed-by: Jason Wang <jasowang@redhat.com>

Applied, thanks (with a 'pci:' subject prefix added).

-- PMM

Re: [Qemu-devel] [PATCH] Fix crash with illegal "-net nic, model=xxx" option

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 09/04/2015 20:31, Eric Blake wrote:
>>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>>> index 6941a82..b3d5100 100644
>>> --- a/hw/pci/pci.c
>>> +++ b/hw/pci/pci.c
>>> @@ -1660,7 +1660,9 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
>>>  
>>>      res = pci_nic_init(nd, rootbus, default_model, default_devaddr, &err);
>>>      if (!res) {
>>> -        error_report_err(err);
>>> +        if (err) {
>>> +            error_report_err(err);
>>> +        }
>>>          exit(1);
>> 
>> Doesn't this mean the program can exit without an error message, if
>> pci_nic_init returns failure but failed to set err?  Shouldn't you at
>> least print something in that case as an else branch?
>
> git grep 'Unsupported NIC model' shows that the error is printed with
> error_report; same for other errors produced by pci_nic_init.
>
> This is not beautiful compared to correct propagation of Error*, but
> it's okay because -net is only used at startup.  It's good enough for rc3.

Thomas, can you clean this up now 2.3 is out?

Re: [Qemu-devel] [PATCH] Fix crash with illegal "-net nic, model=xxx" option

[Thomas Huth]

On Mon, 27 Apr 2015 13:48:27 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
> > On 09/04/2015 20:31, Eric Blake wrote:
> >>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> >>> index 6941a82..b3d5100 100644
> >>> --- a/hw/pci/pci.c
> >>> +++ b/hw/pci/pci.c
> >>> @@ -1660,7 +1660,9 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
> >>>  
> >>>      res = pci_nic_init(nd, rootbus, default_model, default_devaddr, &err);
> >>>      if (!res) {
> >>> -        error_report_err(err);
> >>> +        if (err) {
> >>> +            error_report_err(err);
> >>> +        }
> >>>          exit(1);
> >> 
> >> Doesn't this mean the program can exit without an error message, if
> >> pci_nic_init returns failure but failed to set err?  Shouldn't you at
> >> least print something in that case as an else branch?
> >
> > git grep 'Unsupported NIC model' shows that the error is printed with
> > error_report; same for other errors produced by pci_nic_init.
> >
> > This is not beautiful compared to correct propagation of Error*, but
> > it's okay because -net is only used at startup.  It's good enough for rc3.
> 
> Thomas, can you clean this up now 2.3 is out?

Sure, I'll have a look.

 Thomas