* Re: [PATCH v2] Fixed integer overflow in e1000e
2020-03-04 14:20 [PATCH v2] Fixed integer overflow in e1000e andrew
@ 2020-03-04 15:41 ` Philippe Mathieu-Daudé
2020-03-17 6:20 ` Jason Wang
2020-03-05 9:14 ` Dmitry Fleytman
2020-03-05 9:17 ` Dmitry Fleytman
2 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-04 15:41 UTC (permalink / raw)
To: andrew, dmitry.fleytman; +Cc: jasowang, qemu-devel
On 3/4/20 3:20 PM, andrew@daynix.com wrote:
> From: Andrew Melnychenko <andrew@daynix.com>
>
> Fixes: 6f3fbe4ed06
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1737400
> Fixed setting max_queue_num if there are no peers in NICConf. qemu_new_nic() creates NICState with 1 NetClientState(index 0) without peers, set max_queue_num to 0 - It prevents undefined behavior and possible crashes, especially during pcie hotplug.
Hoping the maintainer taking this can reformat the commit description a
bit nicer... (moving the tags down), then for the code part:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
> hw/net/e1000e.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> index a91dbdca3c..f2cc1552c5 100644
> --- a/hw/net/e1000e.c
> +++ b/hw/net/e1000e.c
> @@ -328,7 +328,7 @@ e1000e_init_net_peer(E1000EState *s, PCIDevice *pci_dev, uint8_t *macaddr)
> s->nic = qemu_new_nic(&net_e1000e_info, &s->conf,
> object_get_typename(OBJECT(s)), dev->id, s);
>
> - s->core.max_queue_num = s->conf.peers.queues - 1;
> + s->core.max_queue_num = s->conf.peers.queues ? s->conf.peers.queues - 1 : 0;
>
> trace_e1000e_mac_set_permanent(MAC_ARG(macaddr));
> memcpy(s->core.permanent_mac, macaddr, sizeof(s->core.permanent_mac));
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Fixed integer overflow in e1000e
2020-03-04 15:41 ` Philippe Mathieu-Daudé
@ 2020-03-17 6:20 ` Jason Wang
0 siblings, 0 replies; 6+ messages in thread
From: Jason Wang @ 2020-03-17 6:20 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, andrew, dmitry.fleytman; +Cc: qemu-devel
On 2020/3/4 下午11:41, Philippe Mathieu-Daudé wrote:
> On 3/4/20 3:20 PM, andrew@daynix.com wrote:
>> From: Andrew Melnychenko <andrew@daynix.com>
>>
>> Fixes: 6f3fbe4ed06
>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1737400
>> Fixed setting max_queue_num if there are no peers in NICConf.
>> qemu_new_nic() creates NICState with 1 NetClientState(index 0)
>> without peers, set max_queue_num to 0 - It prevents undefined
>> behavior and possible crashes, especially during pcie hotplug.
>
> Hoping the maintainer taking this can reformat the commit description
> a bit nicer... (moving the tags down), then for the code part:
Yes, I tweak the log.
Thanks
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
>>
>> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
>> ---
>> hw/net/e1000e.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
>> index a91dbdca3c..f2cc1552c5 100644
>> --- a/hw/net/e1000e.c
>> +++ b/hw/net/e1000e.c
>> @@ -328,7 +328,7 @@ e1000e_init_net_peer(E1000EState *s, PCIDevice
>> *pci_dev, uint8_t *macaddr)
>> s->nic = qemu_new_nic(&net_e1000e_info, &s->conf,
>> object_get_typename(OBJECT(s)), dev->id, s);
>> - s->core.max_queue_num = s->conf.peers.queues - 1;
>> + s->core.max_queue_num = s->conf.peers.queues ?
>> s->conf.peers.queues - 1 : 0;
>> trace_e1000e_mac_set_permanent(MAC_ARG(macaddr));
>> memcpy(s->core.permanent_mac, macaddr,
>> sizeof(s->core.permanent_mac));
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Fixed integer overflow in e1000e
2020-03-04 14:20 [PATCH v2] Fixed integer overflow in e1000e andrew
2020-03-04 15:41 ` Philippe Mathieu-Daudé
@ 2020-03-05 9:14 ` Dmitry Fleytman
2020-03-05 9:17 ` Dmitry Fleytman
2 siblings, 0 replies; 6+ messages in thread
From: Dmitry Fleytman @ 2020-03-05 9:14 UTC (permalink / raw)
To: andrew; +Cc: Jason Wang, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1316 bytes --]
> On 4 Mar 2020, at 16:20, andrew@daynix.com wrote:
>
> From: Andrew Melnychenko <andrew@daynix.com>
>
> Fixes: 6f3fbe4ed06
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1737400
> Fixed setting max_queue_num if there are no peers in NICConf. qemu_new_nic() creates NICState with 1 NetClientState(index 0) without peers, set max_queue_num to 0 - It prevents undefined behavior and possible crashes, especially during pcie hotplug.
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
Reviewed-by: Dmitry Fleytman <dmitry.fleytman@gmail.com>
> ---
> hw/net/e1000e.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> index a91dbdca3c..f2cc1552c5 100644
> --- a/hw/net/e1000e.c
> +++ b/hw/net/e1000e.c
> @@ -328,7 +328,7 @@ e1000e_init_net_peer(E1000EState *s, PCIDevice *pci_dev, uint8_t *macaddr)
> s->nic = qemu_new_nic(&net_e1000e_info, &s->conf,
> object_get_typename(OBJECT(s)), dev->id, s);
>
> - s->core.max_queue_num = s->conf.peers.queues - 1;
> + s->core.max_queue_num = s->conf.peers.queues ? s->conf.peers.queues - 1 : 0;
>
> trace_e1000e_mac_set_permanent(MAC_ARG(macaddr));
> memcpy(s->core.permanent_mac, macaddr, sizeof(s->core.permanent_mac));
> --
> 2.24.1
>
[-- Attachment #2: Type: text/html, Size: 2687 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Fixed integer overflow in e1000e
2020-03-04 14:20 [PATCH v2] Fixed integer overflow in e1000e andrew
2020-03-04 15:41 ` Philippe Mathieu-Daudé
2020-03-05 9:14 ` Dmitry Fleytman
@ 2020-03-05 9:17 ` Dmitry Fleytman
2020-03-17 6:18 ` Jason Wang
2 siblings, 1 reply; 6+ messages in thread
From: Dmitry Fleytman @ 2020-03-05 9:17 UTC (permalink / raw)
To: andrew; +Cc: Jason Wang, qemu-devel
> On 4 Mar 2020, at 16:20, andrew@daynix.com wrote:
>
> From: Andrew Melnychenko <andrew@daynix.com>
>
> Fixes: 6f3fbe4ed06
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1737400
> Fixed setting max_queue_num if there are no peers in NICConf. qemu_new_nic() creates NICState with 1 NetClientState(index 0) without peers, set max_queue_num to 0 - It prevents undefined behavior and possible crashes, especially during pcie hotplug.
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
Reviewed-by: Dmitry Fleytman <dmitry.fleytman@gmail.com>
> ---
> hw/net/e1000e.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> index a91dbdca3c..f2cc1552c5 100644
> --- a/hw/net/e1000e.c
> +++ b/hw/net/e1000e.c
> @@ -328,7 +328,7 @@ e1000e_init_net_peer(E1000EState *s, PCIDevice *pci_dev, uint8_t *macaddr)
> s->nic = qemu_new_nic(&net_e1000e_info, &s->conf,
> object_get_typename(OBJECT(s)), dev->id, s);
>
> - s->core.max_queue_num = s->conf.peers.queues - 1;
> + s->core.max_queue_num = s->conf.peers.queues ? s->conf.peers.queues - 1 : 0;
>
> trace_e1000e_mac_set_permanent(MAC_ARG(macaddr));
> memcpy(s->core.permanent_mac, macaddr, sizeof(s->core.permanent_mac));
> --
> 2.24.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Fixed integer overflow in e1000e
2020-03-05 9:17 ` Dmitry Fleytman
@ 2020-03-17 6:18 ` Jason Wang
0 siblings, 0 replies; 6+ messages in thread
From: Jason Wang @ 2020-03-17 6:18 UTC (permalink / raw)
To: Dmitry Fleytman, andrew; +Cc: qemu-devel
On 2020/3/5 下午5:17, Dmitry Fleytman wrote:
>
>> On 4 Mar 2020, at 16:20, andrew@daynix.com wrote:
>>
>> From: Andrew Melnychenko <andrew@daynix.com>
>>
>> Fixes: 6f3fbe4ed06
>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1737400
>> Fixed setting max_queue_num if there are no peers in NICConf. qemu_new_nic() creates NICState with 1 NetClientState(index 0) without peers, set max_queue_num to 0 - It prevents undefined behavior and possible crashes, especially during pcie hotplug.
>>
>> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> Reviewed-by: Dmitry Fleytman <dmitry.fleytman@gmail.com>
Applied.
Thanks
>
>> ---
>> hw/net/e1000e.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
>> index a91dbdca3c..f2cc1552c5 100644
>> --- a/hw/net/e1000e.c
>> +++ b/hw/net/e1000e.c
>> @@ -328,7 +328,7 @@ e1000e_init_net_peer(E1000EState *s, PCIDevice *pci_dev, uint8_t *macaddr)
>> s->nic = qemu_new_nic(&net_e1000e_info, &s->conf,
>> object_get_typename(OBJECT(s)), dev->id, s);
>>
>> - s->core.max_queue_num = s->conf.peers.queues - 1;
>> + s->core.max_queue_num = s->conf.peers.queues ? s->conf.peers.queues - 1 : 0;
>>
>> trace_e1000e_mac_set_permanent(MAC_ARG(macaddr));
>> memcpy(s->core.permanent_mac, macaddr, sizeof(s->core.permanent_mac));
>> --
>> 2.24.1
>>
^ permalink raw reply [flat|nested] 6+ messages in thread