| Message ID | 1366870889-950-1-git-send-email-jasowang@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Apr 25, 2013 at 02:21:29PM +0800, Jason Wang wrote: > Commit 14f9b664 (hw/virtio-net.c: set config size using host features) tries to > calculate config size based on the host features. But it forgets the > VIRTIO_NET_F_MAC were always set for qemu later. This will lead a zero config > len for virtio-net device when both VIRTIO_NET_F_STATUS and VIRTIO_NET_F_MQ were > disabled form command line. Then qemu will crash when user tries to read the > config of virtio-net. > > Fix this by counting VIRTIO_NET_F_MAC and make sure the config at least contains > the mac address. > > Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com> > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > hw/net/virtio-net.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 70c8fce..33a70ef 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -1264,7 +1264,8 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, > > void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features) > { > - int i, config_size = 0; > + /* VIRTIO_NET_F_MAC can't be disabled from qemu side */ > + int i, config_size = feature_sizes[0].end; This would be cleaner: host_features |= (1 << VIRTIO_NET_F_MAC); no need for a comment then. > for (i = 0; feature_sizes[i].flags != 0; i++) { > if (host_features & feature_sizes[i].flags) { > config_size = MAX(feature_sizes[i].end, config_size); > -- > 1.7.1
On 04/25/2013 02:59 PM, Michael S. Tsirkin wrote: > On Thu, Apr 25, 2013 at 02:21:29PM +0800, Jason Wang wrote: >> Commit 14f9b664 (hw/virtio-net.c: set config size using host features) tries to >> calculate config size based on the host features. But it forgets the >> VIRTIO_NET_F_MAC were always set for qemu later. This will lead a zero config >> len for virtio-net device when both VIRTIO_NET_F_STATUS and VIRTIO_NET_F_MQ were >> disabled form command line. Then qemu will crash when user tries to read the >> config of virtio-net. >> >> Fix this by counting VIRTIO_NET_F_MAC and make sure the config at least contains >> the mac address. >> >> Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com> >> Signed-off-by: Jason Wang <jasowang@redhat.com> >> --- >> hw/net/virtio-net.c | 3 ++- >> 1 files changed, 2 insertions(+), 1 deletions(-) >> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >> index 70c8fce..33a70ef 100644 >> --- a/hw/net/virtio-net.c >> +++ b/hw/net/virtio-net.c >> @@ -1264,7 +1264,8 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, >> >> void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features) >> { >> - int i, config_size = 0; >> + /* VIRTIO_NET_F_MAC can't be disabled from qemu side */ >> + int i, config_size = feature_sizes[0].end; > This would be cleaner: > host_features |= (1 << VIRTIO_NET_F_MAC); > > no need for a comment then. Sure, will send V2. >> for (i = 0; feature_sizes[i].flags != 0; i++) { >> if (host_features & feature_sizes[i].flags) { >> config_size = MAX(feature_sizes[i].end, config_size); >> -- >> 1.7.1
On Thu, Apr 25, 2013 at 03:02:44PM +0800, Jason Wang wrote: > On 04/25/2013 02:59 PM, Michael S. Tsirkin wrote: > > On Thu, Apr 25, 2013 at 02:21:29PM +0800, Jason Wang wrote: > >> Commit 14f9b664 (hw/virtio-net.c: set config size using host features) tries to > >> calculate config size based on the host features. But it forgets the > >> VIRTIO_NET_F_MAC were always set for qemu later. This will lead a zero config > >> len for virtio-net device when both VIRTIO_NET_F_STATUS and VIRTIO_NET_F_MQ were > >> disabled form command line. Then qemu will crash when user tries to read the > >> config of virtio-net. > >> > >> Fix this by counting VIRTIO_NET_F_MAC and make sure the config at least contains > >> the mac address. > >> > >> Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com> > >> Signed-off-by: Jason Wang <jasowang@redhat.com> > >> --- > >> hw/net/virtio-net.c | 3 ++- > >> 1 files changed, 2 insertions(+), 1 deletions(-) > >> > >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > >> index 70c8fce..33a70ef 100644 > >> --- a/hw/net/virtio-net.c > >> +++ b/hw/net/virtio-net.c > >> @@ -1264,7 +1264,8 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, > >> > >> void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features) > >> { > >> - int i, config_size = 0; > >> + /* VIRTIO_NET_F_MAC can't be disabled from qemu side */ > >> + int i, config_size = feature_sizes[0].end; > > This would be cleaner: > > host_features |= (1 << VIRTIO_NET_F_MAC); > > > > no need for a comment then. > > Sure, will send V2. Maybe ass assert(config_size) in core just to make sure other devices do not have this bug. > >> for (i = 0; feature_sizes[i].flags != 0; i++) { > >> if (host_features & feature_sizes[i].flags) { > >> config_size = MAX(feature_sizes[i].end, config_size); > >> -- > >> 1.7.1
On 04/25/2013 03:06 PM, Michael S. Tsirkin wrote: > On Thu, Apr 25, 2013 at 03:02:44PM +0800, Jason Wang wrote: >> On 04/25/2013 02:59 PM, Michael S. Tsirkin wrote: >>> On Thu, Apr 25, 2013 at 02:21:29PM +0800, Jason Wang wrote: >>>> Commit 14f9b664 (hw/virtio-net.c: set config size using host features) tries to >>>> calculate config size based on the host features. But it forgets the >>>> VIRTIO_NET_F_MAC were always set for qemu later. This will lead a zero config >>>> len for virtio-net device when both VIRTIO_NET_F_STATUS and VIRTIO_NET_F_MQ were >>>> disabled form command line. Then qemu will crash when user tries to read the >>>> config of virtio-net. >>>> >>>> Fix this by counting VIRTIO_NET_F_MAC and make sure the config at least contains >>>> the mac address. >>>> >>>> Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com> >>>> Signed-off-by: Jason Wang <jasowang@redhat.com> >>>> --- >>>> hw/net/virtio-net.c | 3 ++- >>>> 1 files changed, 2 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >>>> index 70c8fce..33a70ef 100644 >>>> --- a/hw/net/virtio-net.c >>>> +++ b/hw/net/virtio-net.c >>>> @@ -1264,7 +1264,8 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, >>>> >>>> void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features) >>>> { >>>> - int i, config_size = 0; >>>> + /* VIRTIO_NET_F_MAC can't be disabled from qemu side */ >>>> + int i, config_size = feature_sizes[0].end; >>> This would be cleaner: >>> host_features |= (1 << VIRTIO_NET_F_MAC); >>> >>> no need for a comment then. >> Sure, will send V2. > Maybe ass assert(config_size) in core just to make sure > other devices do not have this bug. Prepare a patch for this, will send soon. >>>> for (i = 0; feature_sizes[i].flags != 0; i++) { >>>> if (host_features & feature_sizes[i].flags) { >>>> config_size = MAX(feature_sizes[i].end, config_size); >>>> -- >>>> 1.7.1
On 04/25/2013 01:59 AM, Michael S. Tsirkin wrote: > On Thu, Apr 25, 2013 at 02:21:29PM +0800, Jason Wang wrote: >> Commit 14f9b664 (hw/virtio-net.c: set config size using host features) tries to >> calculate config size based on the host features. But it forgets the >> VIRTIO_NET_F_MAC were always set for qemu later. This will lead a zero config >> len for virtio-net device when both VIRTIO_NET_F_STATUS and VIRTIO_NET_F_MQ were >> disabled form command line. Then qemu will crash when user tries to read the >> config of virtio-net. >> >> Fix this by counting VIRTIO_NET_F_MAC and make sure the config at least contains >> the mac address. >> >> Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com> >> Signed-off-by: Jason Wang <jasowang@redhat.com> >> --- >> hw/net/virtio-net.c | 3 ++- >> 1 files changed, 2 insertions(+), 1 deletions(-) >> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >> index 70c8fce..33a70ef 100644 >> --- a/hw/net/virtio-net.c >> +++ b/hw/net/virtio-net.c >> @@ -1264,7 +1264,8 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, >> >> void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features) >> { >> - int i, config_size = 0; >> + /* VIRTIO_NET_F_MAC can't be disabled from qemu side */ >> + int i, config_size = feature_sizes[0].end; > > This would be cleaner: > host_features |= (1 << VIRTIO_NET_F_MAC); > > no need for a comment then. > It seems to me that the real problem here is that host_features isn't properly populated before virtio_net_set_config_size() is called. Looking at virtio_device_init(), we can see why: static int virtio_device_init(DeviceState *qdev) { VirtIODevice *vdev = VIRTIO_DEVICE(qdev); VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); assert(k->init != NULL); if (k->init(vdev) < 0) { return -1; } virtio_bus_plug_device(vdev); return 0; } virtio_net_set_config_size() is currently being called as part of the k->init call, but the host_features aren't properly setup until the device is plugged into the bus using virtio_bus_plug_device(). After talking with mdroth, I think the proper way to fix this would be to extend VirtioDeviceClass to include a calculate_config_size() method that can be called at the appropriate time during device initialization like so: static int virtio_device_init(DeviceState *qdev) { VirtIODevice *vdev = VIRTIO_DEVICE(qdev); VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); assert(k->init != NULL); if (k->init(vdev) < 0) { return -1; } virtio_bus_plug_device(vdev); + if (k->calculate_config_size && k->calculate_config_size(vdev) < 0) { + return -1; + } return 0; } This would ensure that host_features contains the proper data before any devices try to make use of it to calculate the config size. >> for (i = 0; feature_sizes[i].flags != 0; i++) { >> if (host_features & feature_sizes[i].flags) { >> config_size = MAX(feature_sizes[i].end, config_size); >> -- >> 1.7.1 > Jesse Larrew Software Engineer, KVM Team IBM Linux Technology Center Phone: (512) 973-2052 (T/L: 363-2052) jlarrew@linux.vnet.ibm.com
On 29/04/2013 16:42, Jesse Larrew wrote: > On 04/25/2013 01:59 AM, Michael S. Tsirkin wrote: >> On Thu, Apr 25, 2013 at 02:21:29PM +0800, Jason Wang wrote: >>> Commit 14f9b664 (hw/virtio-net.c: set config size using host features) tries to >>> calculate config size based on the host features. But it forgets the >>> VIRTIO_NET_F_MAC were always set for qemu later. This will lead a zero config >>> len for virtio-net device when both VIRTIO_NET_F_STATUS and VIRTIO_NET_F_MQ were >>> disabled form command line. Then qemu will crash when user tries to read the >>> config of virtio-net. >>> >>> Fix this by counting VIRTIO_NET_F_MAC and make sure the config at least contains >>> the mac address. >>> >>> Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com> >>> Signed-off-by: Jason Wang <jasowang@redhat.com> >>> --- >>> hw/net/virtio-net.c | 3 ++- >>> 1 files changed, 2 insertions(+), 1 deletions(-) >>> >>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >>> index 70c8fce..33a70ef 100644 >>> --- a/hw/net/virtio-net.c >>> +++ b/hw/net/virtio-net.c >>> @@ -1264,7 +1264,8 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, >>> >>> void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features) >>> { >>> - int i, config_size = 0; >>> + /* VIRTIO_NET_F_MAC can't be disabled from qemu side */ >>> + int i, config_size = feature_sizes[0].end; >> This would be cleaner: >> host_features |= (1 << VIRTIO_NET_F_MAC); >> >> no need for a comment then. >> > It seems to me that the real problem here is that host_features isn't > properly populated before virtio_net_set_config_size() is called. Looking > at virtio_device_init(), we can see why: > > static int virtio_device_init(DeviceState *qdev) > { > VirtIODevice *vdev = VIRTIO_DEVICE(qdev); > VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); > assert(k->init != NULL); > if (k->init(vdev) < 0) { > return -1; > } > virtio_bus_plug_device(vdev); > return 0; > } > > virtio_net_set_config_size() is currently being called as part of the > k->init call, but the host_features aren't properly setup until the device > is plugged into the bus using virtio_bus_plug_device(). > > After talking with mdroth, I think the proper way to fix this would be to > extend VirtioDeviceClass to include a calculate_config_size() method that > can be called at the appropriate time during device initialization like so: > > static int virtio_device_init(DeviceState *qdev) > { > VirtIODevice *vdev = VIRTIO_DEVICE(qdev); > VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); > assert(k->init != NULL); > if (k->init(vdev) < 0) { > return -1; > } > virtio_bus_plug_device(vdev); > + if (k->calculate_config_size && k->calculate_config_size(vdev) < 0) { > + return -1; > + } > return 0; > } > > This would ensure that host_features contains the proper data before any > devices try to make use of it to calculate the config size. Good point, I didn't saw that. but this was not the case with commit 14f9b664 no? > >>> for (i = 0; feature_sizes[i].flags != 0; i++) { >>> if (host_features & feature_sizes[i].flags) { >>> config_size = MAX(feature_sizes[i].end, config_size); >>> -- >>> 1.7.1 > Jesse Larrew > Software Engineer, KVM Team > IBM Linux Technology Center > Phone: (512) 973-2052 (T/L: 363-2052) > jlarrew@linux.vnet.ibm.com > >
On 04/29/2013 09:55 AM, KONRAD Frédéric wrote: > On 29/04/2013 16:42, Jesse Larrew wrote: >> On 04/25/2013 01:59 AM, Michael S. Tsirkin wrote: >>> On Thu, Apr 25, 2013 at 02:21:29PM +0800, Jason Wang wrote: >>>> Commit 14f9b664 (hw/virtio-net.c: set config size using host features) tries to >>>> calculate config size based on the host features. But it forgets the >>>> VIRTIO_NET_F_MAC were always set for qemu later. This will lead a zero config >>>> len for virtio-net device when both VIRTIO_NET_F_STATUS and VIRTIO_NET_F_MQ were >>>> disabled form command line. Then qemu will crash when user tries to read the >>>> config of virtio-net. >>>> >>>> Fix this by counting VIRTIO_NET_F_MAC and make sure the config at least contains >>>> the mac address. >>>> >>>> Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com> >>>> Signed-off-by: Jason Wang <jasowang@redhat.com> >>>> --- >>>> hw/net/virtio-net.c | 3 ++- >>>> 1 files changed, 2 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >>>> index 70c8fce..33a70ef 100644 >>>> --- a/hw/net/virtio-net.c >>>> +++ b/hw/net/virtio-net.c >>>> @@ -1264,7 +1264,8 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, >>>> void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features) >>>> { >>>> - int i, config_size = 0; >>>> + /* VIRTIO_NET_F_MAC can't be disabled from qemu side */ >>>> + int i, config_size = feature_sizes[0].end; >>> This would be cleaner: >>> host_features |= (1 << VIRTIO_NET_F_MAC); >>> >>> no need for a comment then. >>> >> It seems to me that the real problem here is that host_features isn't >> properly populated before virtio_net_set_config_size() is called. Looking >> at virtio_device_init(), we can see why: >> >> static int virtio_device_init(DeviceState *qdev) >> { >> VirtIODevice *vdev = VIRTIO_DEVICE(qdev); >> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); >> assert(k->init != NULL); >> if (k->init(vdev) < 0) { >> return -1; >> } >> virtio_bus_plug_device(vdev); >> return 0; >> } >> >> virtio_net_set_config_size() is currently being called as part of the >> k->init call, but the host_features aren't properly setup until the device >> is plugged into the bus using virtio_bus_plug_device(). >> >> After talking with mdroth, I think the proper way to fix this would be to >> extend VirtioDeviceClass to include a calculate_config_size() method that >> can be called at the appropriate time during device initialization like so: >> >> static int virtio_device_init(DeviceState *qdev) >> { >> VirtIODevice *vdev = VIRTIO_DEVICE(qdev); >> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); >> assert(k->init != NULL); >> if (k->init(vdev) < 0) { >> return -1; >> } >> virtio_bus_plug_device(vdev); >> + if (k->calculate_config_size && k->calculate_config_size(vdev) < 0) { >> + return -1; >> + } >> return 0; >> } >> >> This would ensure that host_features contains the proper data before any >> devices try to make use of it to calculate the config size. > Good point, I didn't saw that. > > but this was not the case with commit 14f9b664 no? > I suspect this bug was present in 14f9b664 as well. We just hadn't triggered it yet. I'll confirm this afternoon. >> >>>> for (i = 0; feature_sizes[i].flags != 0; i++) { >>>> if (host_features & feature_sizes[i].flags) { >>>> config_size = MAX(feature_sizes[i].end, config_size); >>>> -- >>>> 1.7.1 Jesse Larrew Software Engineer, KVM Team IBM Linux Technology Center Phone: (512) 973-2052 (T/L: 363-2052) jlarrew@linux.vnet.ibm.com
On 29/04/2013 17:14, Jesse Larrew wrote: > On 04/29/2013 09:55 AM, KONRAD Frédéric wrote: >> On 29/04/2013 16:42, Jesse Larrew wrote: >>> On 04/25/2013 01:59 AM, Michael S. Tsirkin wrote: >>>> On Thu, Apr 25, 2013 at 02:21:29PM +0800, Jason Wang wrote: >>>>> Commit 14f9b664 (hw/virtio-net.c: set config size using host features) tries to >>>>> calculate config size based on the host features. But it forgets the >>>>> VIRTIO_NET_F_MAC were always set for qemu later. This will lead a zero config >>>>> len for virtio-net device when both VIRTIO_NET_F_STATUS and VIRTIO_NET_F_MQ were >>>>> disabled form command line. Then qemu will crash when user tries to read the >>>>> config of virtio-net. >>>>> >>>>> Fix this by counting VIRTIO_NET_F_MAC and make sure the config at least contains >>>>> the mac address. >>>>> >>>>> Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com> >>>>> Signed-off-by: Jason Wang <jasowang@redhat.com> >>>>> --- >>>>> hw/net/virtio-net.c | 3 ++- >>>>> 1 files changed, 2 insertions(+), 1 deletions(-) >>>>> >>>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >>>>> index 70c8fce..33a70ef 100644 >>>>> --- a/hw/net/virtio-net.c >>>>> +++ b/hw/net/virtio-net.c >>>>> @@ -1264,7 +1264,8 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, >>>>> void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features) >>>>> { >>>>> - int i, config_size = 0; >>>>> + /* VIRTIO_NET_F_MAC can't be disabled from qemu side */ >>>>> + int i, config_size = feature_sizes[0].end; >>>> This would be cleaner: >>>> host_features |= (1 << VIRTIO_NET_F_MAC); >>>> >>>> no need for a comment then. >>>> >>> It seems to me that the real problem here is that host_features isn't >>> properly populated before virtio_net_set_config_size() is called. Looking >>> at virtio_device_init(), we can see why: >>> >>> static int virtio_device_init(DeviceState *qdev) >>> { >>> VirtIODevice *vdev = VIRTIO_DEVICE(qdev); >>> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); >>> assert(k->init != NULL); >>> if (k->init(vdev) < 0) { >>> return -1; >>> } >>> virtio_bus_plug_device(vdev); >>> return 0; >>> } >>> >>> virtio_net_set_config_size() is currently being called as part of the >>> k->init call, but the host_features aren't properly setup until the device >>> is plugged into the bus using virtio_bus_plug_device(). >>> >>> After talking with mdroth, I think the proper way to fix this would be to >>> extend VirtioDeviceClass to include a calculate_config_size() method that >>> can be called at the appropriate time during device initialization like so: >>> >>> static int virtio_device_init(DeviceState *qdev) >>> { >>> VirtIODevice *vdev = VIRTIO_DEVICE(qdev); >>> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); >>> assert(k->init != NULL); >>> if (k->init(vdev) < 0) { >>> return -1; >>> } >>> virtio_bus_plug_device(vdev); >>> + if (k->calculate_config_size && k->calculate_config_size(vdev) < 0) { >>> + return -1; >>> + } >>> return 0; >>> } >>> >>> This would ensure that host_features contains the proper data before any >>> devices try to make use of it to calculate the config size. >> Good point, I didn't saw that. >> >> but this was not the case with commit 14f9b664 no? >> > I suspect this bug was present in 14f9b664 as well. We just hadn't triggered > it yet. I'll confirm this afternoon. Ok, I think there is a problem with your patch: virtio_init(VIRTIO_DEVICE(n), "virtio-net", VIRTIO_ID_NET, n->config_size); is called in virtio_net_device_init (k->init). Is there a way to resize the config after that? > >>>>> for (i = 0; feature_sizes[i].flags != 0; i++) { >>>>> if (host_features & feature_sizes[i].flags) { >>>>> config_size = MAX(feature_sizes[i].end, config_size); >>>>> -- >>>>> 1.7.1 > Jesse Larrew > Software Engineer, KVM Team > IBM Linux Technology Center > Phone: (512) 973-2052 (T/L: 363-2052) > jlarrew@linux.vnet.ibm.com >
On 04/29/2013 10:29 AM, KONRAD Frédéric wrote: > On 29/04/2013 17:14, Jesse Larrew wrote: >> On 04/29/2013 09:55 AM, KONRAD Frédéric wrote: >>> On 29/04/2013 16:42, Jesse Larrew wrote: >>>> On 04/25/2013 01:59 AM, Michael S. Tsirkin wrote: >>>>> On Thu, Apr 25, 2013 at 02:21:29PM +0800, Jason Wang wrote: >>>>>> Commit 14f9b664 (hw/virtio-net.c: set config size using host features) tries to >>>>>> calculate config size based on the host features. But it forgets the >>>>>> VIRTIO_NET_F_MAC were always set for qemu later. This will lead a zero config >>>>>> len for virtio-net device when both VIRTIO_NET_F_STATUS and VIRTIO_NET_F_MQ were >>>>>> disabled form command line. Then qemu will crash when user tries to read the >>>>>> config of virtio-net. >>>>>> >>>>>> Fix this by counting VIRTIO_NET_F_MAC and make sure the config at least contains >>>>>> the mac address. >>>>>> >>>>>> Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com> >>>>>> Signed-off-by: Jason Wang <jasowang@redhat.com> >>>>>> --- >>>>>> hw/net/virtio-net.c | 3 ++- >>>>>> 1 files changed, 2 insertions(+), 1 deletions(-) >>>>>> >>>>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >>>>>> index 70c8fce..33a70ef 100644 >>>>>> --- a/hw/net/virtio-net.c >>>>>> +++ b/hw/net/virtio-net.c >>>>>> @@ -1264,7 +1264,8 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, >>>>>> void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features) >>>>>> { >>>>>> - int i, config_size = 0; >>>>>> + /* VIRTIO_NET_F_MAC can't be disabled from qemu side */ >>>>>> + int i, config_size = feature_sizes[0].end; >>>>> This would be cleaner: >>>>> host_features |= (1 << VIRTIO_NET_F_MAC); >>>>> >>>>> no need for a comment then. >>>>> >>>> It seems to me that the real problem here is that host_features isn't >>>> properly populated before virtio_net_set_config_size() is called. Looking >>>> at virtio_device_init(), we can see why: >>>> >>>> static int virtio_device_init(DeviceState *qdev) >>>> { >>>> VirtIODevice *vdev = VIRTIO_DEVICE(qdev); >>>> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); >>>> assert(k->init != NULL); >>>> if (k->init(vdev) < 0) { >>>> return -1; >>>> } >>>> virtio_bus_plug_device(vdev); >>>> return 0; >>>> } >>>> >>>> virtio_net_set_config_size() is currently being called as part of the >>>> k->init call, but the host_features aren't properly setup until the device >>>> is plugged into the bus using virtio_bus_plug_device(). >>>> >>>> After talking with mdroth, I think the proper way to fix this would be to >>>> extend VirtioDeviceClass to include a calculate_config_size() method that >>>> can be called at the appropriate time during device initialization like so: >>>> >>>> static int virtio_device_init(DeviceState *qdev) >>>> { >>>> VirtIODevice *vdev = VIRTIO_DEVICE(qdev); >>>> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); >>>> assert(k->init != NULL); >>>> if (k->init(vdev) < 0) { >>>> return -1; >>>> } >>>> virtio_bus_plug_device(vdev); >>>> + if (k->calculate_config_size && k->calculate_config_size(vdev) < 0) { >>>> + return -1; >>>> + } >>>> return 0; >>>> } >>>> >>>> This would ensure that host_features contains the proper data before any >>>> devices try to make use of it to calculate the config size. >>> Good point, I didn't saw that. >>> >>> but this was not the case with commit 14f9b664 no? >>> >> I suspect this bug was present in 14f9b664 as well. We just hadn't triggered >> it yet. I'll confirm this afternoon. > > Ok, I think there is a problem with your patch: > > virtio_init(VIRTIO_DEVICE(n), "virtio-net", VIRTIO_ID_NET, > n->config_size); > > is called in virtio_net_device_init (k->init). > > Is there a way to resize the config after that? > Nope. That's definitely a bug. I can send a patch to fix this today. Any objection to the method suggested above (extending VirtioDeviceClass)? >> >>>>>> for (i = 0; feature_sizes[i].flags != 0; i++) { >>>>>> if (host_features & feature_sizes[i].flags) { >>>>>> config_size = MAX(feature_sizes[i].end, config_size); >>>>>> -- >>>>>> 1.7.1 >> Jesse Larrew >> Software Engineer, KVM Team >> IBM Linux Technology Center >> Phone: (512) 973-2052 (T/L: 363-2052) >> jlarrew@linux.vnet.ibm.com >> >
On Mon, Apr 29, 2013 at 10:55:36AM -0500, Jesse Larrew wrote: > On 04/29/2013 10:29 AM, KONRAD Frédéric wrote: > > On 29/04/2013 17:14, Jesse Larrew wrote: > >> On 04/29/2013 09:55 AM, KONRAD Frédéric wrote: > >>> On 29/04/2013 16:42, Jesse Larrew wrote: > >>>> On 04/25/2013 01:59 AM, Michael S. Tsirkin wrote: > >>>>> On Thu, Apr 25, 2013 at 02:21:29PM +0800, Jason Wang wrote: > >>>>>> Commit 14f9b664 (hw/virtio-net.c: set config size using host features) tries to > >>>>>> calculate config size based on the host features. But it forgets the > >>>>>> VIRTIO_NET_F_MAC were always set for qemu later. This will lead a zero config > >>>>>> len for virtio-net device when both VIRTIO_NET_F_STATUS and VIRTIO_NET_F_MQ were > >>>>>> disabled form command line. Then qemu will crash when user tries to read the > >>>>>> config of virtio-net. > >>>>>> > >>>>>> Fix this by counting VIRTIO_NET_F_MAC and make sure the config at least contains > >>>>>> the mac address. > >>>>>> > >>>>>> Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com> > >>>>>> Signed-off-by: Jason Wang <jasowang@redhat.com> > >>>>>> --- > >>>>>> hw/net/virtio-net.c | 3 ++- > >>>>>> 1 files changed, 2 insertions(+), 1 deletions(-) > >>>>>> > >>>>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > >>>>>> index 70c8fce..33a70ef 100644 > >>>>>> --- a/hw/net/virtio-net.c > >>>>>> +++ b/hw/net/virtio-net.c > >>>>>> @@ -1264,7 +1264,8 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, > >>>>>> void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features) > >>>>>> { > >>>>>> - int i, config_size = 0; > >>>>>> + /* VIRTIO_NET_F_MAC can't be disabled from qemu side */ > >>>>>> + int i, config_size = feature_sizes[0].end; > >>>>> This would be cleaner: > >>>>> host_features |= (1 << VIRTIO_NET_F_MAC); > >>>>> > >>>>> no need for a comment then. > >>>>> > >>>> It seems to me that the real problem here is that host_features isn't > >>>> properly populated before virtio_net_set_config_size() is called. Looking > >>>> at virtio_device_init(), we can see why: > >>>> > >>>> static int virtio_device_init(DeviceState *qdev) > >>>> { > >>>> VirtIODevice *vdev = VIRTIO_DEVICE(qdev); > >>>> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); > >>>> assert(k->init != NULL); > >>>> if (k->init(vdev) < 0) { > >>>> return -1; > >>>> } > >>>> virtio_bus_plug_device(vdev); > >>>> return 0; > >>>> } > >>>> > >>>> virtio_net_set_config_size() is currently being called as part of the > >>>> k->init call, but the host_features aren't properly setup until the device > >>>> is plugged into the bus using virtio_bus_plug_device(). > >>>> > >>>> After talking with mdroth, I think the proper way to fix this would be to > >>>> extend VirtioDeviceClass to include a calculate_config_size() method that > >>>> can be called at the appropriate time during device initialization like so: > >>>> > >>>> static int virtio_device_init(DeviceState *qdev) > >>>> { > >>>> VirtIODevice *vdev = VIRTIO_DEVICE(qdev); > >>>> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); > >>>> assert(k->init != NULL); > >>>> if (k->init(vdev) < 0) { > >>>> return -1; > >>>> } > >>>> virtio_bus_plug_device(vdev); > >>>> + if (k->calculate_config_size && k->calculate_config_size(vdev) < 0) { > >>>> + return -1; > >>>> + } > >>>> return 0; > >>>> } > >>>> > >>>> This would ensure that host_features contains the proper data before any > >>>> devices try to make use of it to calculate the config size. > >>> Good point, I didn't saw that. > >>> > >>> but this was not the case with commit 14f9b664 no? > >>> > >> I suspect this bug was present in 14f9b664 as well. We just hadn't triggered > >> it yet. I'll confirm this afternoon. > > > > Ok, I think there is a problem with your patch: > > > > virtio_init(VIRTIO_DEVICE(n), "virtio-net", VIRTIO_ID_NET, > > n->config_size); > > > > is called in virtio_net_device_init (k->init). > > > > Is there a way to resize the config after that? > > > > Nope. That's definitely a bug. I can send a patch to fix this today. Any > objection to the method suggested above (extending VirtioDeviceClass)? This needs more thought. All devices expected everything is setup during the init call. config size is likely not the only issue. So we need almost all of virtio_bus_plug_device before init, and then after init send the signal: if (klass->device_plugged != NULL) { klass->device_plugged(qbus->parent); } > >> > >>>>>> for (i = 0; feature_sizes[i].flags != 0; i++) { > >>>>>> if (host_features & feature_sizes[i].flags) { > >>>>>> config_size = MAX(feature_sizes[i].end, config_size); > >>>>>> -- > >>>>>> 1.7.1 > >> Jesse Larrew > >> Software Engineer, KVM Team > >> IBM Linux Technology Center > >> Phone: (512) 973-2052 (T/L: 363-2052) > >> jlarrew@linux.vnet.ibm.com > >> > > > > > -- > > Jesse Larrew > Software Engineer, KVM Team > IBM Linux Technology Center > Phone: (512) 973-2052 (T/L: 363-2052) > jlarrew@linux.vnet.ibm.com
On 29/04/2013 18:02, Michael S. Tsirkin wrote: > On Mon, Apr 29, 2013 at 10:55:36AM -0500, Jesse Larrew wrote: >> On 04/29/2013 10:29 AM, KONRAD Frédéric wrote: >>> On 29/04/2013 17:14, Jesse Larrew wrote: >>>> On 04/29/2013 09:55 AM, KONRAD Frédéric wrote: >>>>> On 29/04/2013 16:42, Jesse Larrew wrote: >>>>>> On 04/25/2013 01:59 AM, Michael S. Tsirkin wrote: >>>>>>> On Thu, Apr 25, 2013 at 02:21:29PM +0800, Jason Wang wrote: >>>>>>>> Commit 14f9b664 (hw/virtio-net.c: set config size using host features) tries to >>>>>>>> calculate config size based on the host features. But it forgets the >>>>>>>> VIRTIO_NET_F_MAC were always set for qemu later. This will lead a zero config >>>>>>>> len for virtio-net device when both VIRTIO_NET_F_STATUS and VIRTIO_NET_F_MQ were >>>>>>>> disabled form command line. Then qemu will crash when user tries to read the >>>>>>>> config of virtio-net. >>>>>>>> >>>>>>>> Fix this by counting VIRTIO_NET_F_MAC and make sure the config at least contains >>>>>>>> the mac address. >>>>>>>> >>>>>>>> Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com> >>>>>>>> Signed-off-by: Jason Wang <jasowang@redhat.com> >>>>>>>> --- >>>>>>>> hw/net/virtio-net.c | 3 ++- >>>>>>>> 1 files changed, 2 insertions(+), 1 deletions(-) >>>>>>>> >>>>>>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >>>>>>>> index 70c8fce..33a70ef 100644 >>>>>>>> --- a/hw/net/virtio-net.c >>>>>>>> +++ b/hw/net/virtio-net.c >>>>>>>> @@ -1264,7 +1264,8 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, >>>>>>>> void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features) >>>>>>>> { >>>>>>>> - int i, config_size = 0; >>>>>>>> + /* VIRTIO_NET_F_MAC can't be disabled from qemu side */ >>>>>>>> + int i, config_size = feature_sizes[0].end; >>>>>>> This would be cleaner: >>>>>>> host_features |= (1 << VIRTIO_NET_F_MAC); >>>>>>> >>>>>>> no need for a comment then. >>>>>>> >>>>>> It seems to me that the real problem here is that host_features isn't >>>>>> properly populated before virtio_net_set_config_size() is called. Looking >>>>>> at virtio_device_init(), we can see why: >>>>>> >>>>>> static int virtio_device_init(DeviceState *qdev) >>>>>> { >>>>>> VirtIODevice *vdev = VIRTIO_DEVICE(qdev); >>>>>> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); >>>>>> assert(k->init != NULL); >>>>>> if (k->init(vdev) < 0) { >>>>>> return -1; >>>>>> } >>>>>> virtio_bus_plug_device(vdev); >>>>>> return 0; >>>>>> } >>>>>> >>>>>> virtio_net_set_config_size() is currently being called as part of the >>>>>> k->init call, but the host_features aren't properly setup until the device >>>>>> is plugged into the bus using virtio_bus_plug_device(). >>>>>> >>>>>> After talking with mdroth, I think the proper way to fix this would be to >>>>>> extend VirtioDeviceClass to include a calculate_config_size() method that >>>>>> can be called at the appropriate time during device initialization like so: >>>>>> >>>>>> static int virtio_device_init(DeviceState *qdev) >>>>>> { >>>>>> VirtIODevice *vdev = VIRTIO_DEVICE(qdev); >>>>>> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); >>>>>> assert(k->init != NULL); >>>>>> if (k->init(vdev) < 0) { >>>>>> return -1; >>>>>> } >>>>>> virtio_bus_plug_device(vdev); >>>>>> + if (k->calculate_config_size && k->calculate_config_size(vdev) < 0) { >>>>>> + return -1; >>>>>> + } >>>>>> return 0; >>>>>> } >>>>>> >>>>>> This would ensure that host_features contains the proper data before any >>>>>> devices try to make use of it to calculate the config size. >>>>> Good point, I didn't saw that. >>>>> >>>>> but this was not the case with commit 14f9b664 no? >>>>> >>>> I suspect this bug was present in 14f9b664 as well. We just hadn't triggered >>>> it yet. I'll confirm this afternoon. >>> Ok, I think there is a problem with your patch: >>> >>> virtio_init(VIRTIO_DEVICE(n), "virtio-net", VIRTIO_ID_NET, >>> n->config_size); >>> >>> is called in virtio_net_device_init (k->init). >>> >>> Is there a way to resize the config after that? >>> >> Nope. That's definitely a bug. I can send a patch to fix this today. Any >> objection to the method suggested above (extending VirtioDeviceClass)? > This needs more thought. All devices expected everything > is setup during the init call. config size is > likely not the only issue. > > So we need almost all of virtio_bus_plug_device before init, > and then after init send the signal: > > if (klass->device_plugged != NULL) { > klass->device_plugged(qbus->parent); > } Seems the interesting part is in virtio_pci_device_plugged at the end: proxy->host_features |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY; proxy->host_features |= 0x1 << VIRTIO_F_BAD_FEATURE; proxy->host_features = virtio_bus_get_vdev_features(bus, proxy->host_features); This is and was called after set_config_size, isn't that the issue? > > >>>>>>>> for (i = 0; feature_sizes[i].flags != 0; i++) { >>>>>>>> if (host_features & feature_sizes[i].flags) { >>>>>>>> config_size = MAX(feature_sizes[i].end, config_size); >>>>>>>> -- >>>>>>>> 1.7.1 >>>> Jesse Larrew >>>> Software Engineer, KVM Team >>>> IBM Linux Technology Center >>>> Phone: (512) 973-2052 (T/L: 363-2052) >>>> jlarrew@linux.vnet.ibm.com >>>> >> >> -- >> >> Jesse Larrew >> Software Engineer, KVM Team >> IBM Linux Technology Center >> Phone: (512) 973-2052 (T/L: 363-2052) >> jlarrew@linux.vnet.ibm.com
On Mon, Apr 29, 2013 at 06:14:41PM +0200, KONRAD Frédéric wrote: > On 29/04/2013 18:02, Michael S. Tsirkin wrote: > >On Mon, Apr 29, 2013 at 10:55:36AM -0500, Jesse Larrew wrote: > >>On 04/29/2013 10:29 AM, KONRAD Frédéric wrote: > >>>On 29/04/2013 17:14, Jesse Larrew wrote: > >>>>On 04/29/2013 09:55 AM, KONRAD Frédéric wrote: > >>>>>On 29/04/2013 16:42, Jesse Larrew wrote: > >>>>>>On 04/25/2013 01:59 AM, Michael S. Tsirkin wrote: > >>>>>>>On Thu, Apr 25, 2013 at 02:21:29PM +0800, Jason Wang wrote: > >>>>>>>>Commit 14f9b664 (hw/virtio-net.c: set config size using host features) tries to > >>>>>>>>calculate config size based on the host features. But it forgets the > >>>>>>>>VIRTIO_NET_F_MAC were always set for qemu later. This will lead a zero config > >>>>>>>>len for virtio-net device when both VIRTIO_NET_F_STATUS and VIRTIO_NET_F_MQ were > >>>>>>>>disabled form command line. Then qemu will crash when user tries to read the > >>>>>>>>config of virtio-net. > >>>>>>>> > >>>>>>>>Fix this by counting VIRTIO_NET_F_MAC and make sure the config at least contains > >>>>>>>>the mac address. > >>>>>>>> > >>>>>>>>Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com> > >>>>>>>>Signed-off-by: Jason Wang <jasowang@redhat.com> > >>>>>>>>--- > >>>>>>>> hw/net/virtio-net.c | 3 ++- > >>>>>>>> 1 files changed, 2 insertions(+), 1 deletions(-) > >>>>>>>> > >>>>>>>>diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > >>>>>>>>index 70c8fce..33a70ef 100644 > >>>>>>>>--- a/hw/net/virtio-net.c > >>>>>>>>+++ b/hw/net/virtio-net.c > >>>>>>>>@@ -1264,7 +1264,8 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, > >>>>>>>> void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features) > >>>>>>>> { > >>>>>>>>- int i, config_size = 0; > >>>>>>>>+ /* VIRTIO_NET_F_MAC can't be disabled from qemu side */ > >>>>>>>>+ int i, config_size = feature_sizes[0].end; > >>>>>>>This would be cleaner: > >>>>>>> host_features |= (1 << VIRTIO_NET_F_MAC); > >>>>>>> > >>>>>>>no need for a comment then. > >>>>>>> > >>>>>>It seems to me that the real problem here is that host_features isn't > >>>>>>properly populated before virtio_net_set_config_size() is called. Looking > >>>>>>at virtio_device_init(), we can see why: > >>>>>> > >>>>>>static int virtio_device_init(DeviceState *qdev) > >>>>>>{ > >>>>>> VirtIODevice *vdev = VIRTIO_DEVICE(qdev); > >>>>>> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); > >>>>>> assert(k->init != NULL); > >>>>>> if (k->init(vdev) < 0) { > >>>>>> return -1; > >>>>>> } > >>>>>> virtio_bus_plug_device(vdev); > >>>>>> return 0; > >>>>>>} > >>>>>> > >>>>>>virtio_net_set_config_size() is currently being called as part of the > >>>>>>k->init call, but the host_features aren't properly setup until the device > >>>>>>is plugged into the bus using virtio_bus_plug_device(). > >>>>>> > >>>>>>After talking with mdroth, I think the proper way to fix this would be to > >>>>>>extend VirtioDeviceClass to include a calculate_config_size() method that > >>>>>>can be called at the appropriate time during device initialization like so: > >>>>>> > >>>>>>static int virtio_device_init(DeviceState *qdev) > >>>>>>{ > >>>>>> VirtIODevice *vdev = VIRTIO_DEVICE(qdev); > >>>>>> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev); > >>>>>> assert(k->init != NULL); > >>>>>> if (k->init(vdev) < 0) { > >>>>>> return -1; > >>>>>> } > >>>>>> virtio_bus_plug_device(vdev); > >>>>>>+ if (k->calculate_config_size && k->calculate_config_size(vdev) < 0) { > >>>>>>+ return -1; > >>>>>>+ } > >>>>>> return 0; > >>>>>>} > >>>>>> > >>>>>>This would ensure that host_features contains the proper data before any > >>>>>>devices try to make use of it to calculate the config size. > >>>>>Good point, I didn't saw that. > >>>>> > >>>>>but this was not the case with commit 14f9b664 no? > >>>>> > >>>>I suspect this bug was present in 14f9b664 as well. We just hadn't triggered > >>>>it yet. I'll confirm this afternoon. > >>>Ok, I think there is a problem with your patch: > >>> > >>> virtio_init(VIRTIO_DEVICE(n), "virtio-net", VIRTIO_ID_NET, > >>> n->config_size); > >>> > >>>is called in virtio_net_device_init (k->init). > >>> > >>>Is there a way to resize the config after that? > >>> > >>Nope. That's definitely a bug. I can send a patch to fix this today. Any > >>objection to the method suggested above (extending VirtioDeviceClass)? > >This needs more thought. All devices expected everything > >is setup during the init call. config size is > >likely not the only issue. > > > >So we need almost all of virtio_bus_plug_device before init, > >and then after init send the signal: > > > > if (klass->device_plugged != NULL) { > > klass->device_plugged(qbus->parent); > > } > > Seems the interesting part is in virtio_pci_device_plugged at the end: > > proxy->host_features |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY; > proxy->host_features |= 0x1 << VIRTIO_F_BAD_FEATURE; > proxy->host_features = virtio_bus_get_vdev_features(bus, > proxy->host_features); > > This is and was called after set_config_size, isn't that the issue? Basically devices expected everything to be setup at the point where init is called. We found one bug but let's fix it properly. There's no reason to delay parts of setup until after init, if we do, we'll trip on more uses of uninitialized data. > > > > > >>>>>>>> for (i = 0; feature_sizes[i].flags != 0; i++) { > >>>>>>>> if (host_features & feature_sizes[i].flags) { > >>>>>>>> config_size = MAX(feature_sizes[i].end, config_size); > >>>>>>>>-- > >>>>>>>>1.7.1 > >>>>Jesse Larrew > >>>>Software Engineer, KVM Team > >>>>IBM Linux Technology Center > >>>>Phone: (512) 973-2052 (T/L: 363-2052) > >>>>jlarrew@linux.vnet.ibm.com > >>>> > >> > >>-- > >> > >>Jesse Larrew > >>Software Engineer, KVM Team > >>IBM Linux Technology Center > >>Phone: (512) 973-2052 (T/L: 363-2052) > >>jlarrew@linux.vnet.ibm.com
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 70c8fce..33a70ef 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -1264,7 +1264,8 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features) { - int i, config_size = 0; + /* VIRTIO_NET_F_MAC can't be disabled from qemu side */ + int i, config_size = feature_sizes[0].end; for (i = 0; feature_sizes[i].flags != 0; i++) { if (host_features & feature_sizes[i].flags) { config_size = MAX(feature_sizes[i].end, config_size);
Commit 14f9b664 (hw/virtio-net.c: set config size using host features) tries to calculate config size based on the host features. But it forgets the VIRTIO_NET_F_MAC were always set for qemu later. This will lead a zero config len for virtio-net device when both VIRTIO_NET_F_STATUS and VIRTIO_NET_F_MQ were disabled form command line. Then qemu will crash when user tries to read the config of virtio-net. Fix this by counting VIRTIO_NET_F_MAC and make sure the config at least contains the mac address. Cc: Jesse Larrew <jlarrew@linux.vnet.ibm.com> Signed-off-by: Jason Wang <jasowang@redhat.com> --- hw/net/virtio-net.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)