* [Qemu-devel] [PATCH] vmxnet3: Do not fill stats if device is inactive
@ 2015-10-15 10:54 Dana Rubin
2015-10-18 7:16 ` Dmitry Fleytman
2015-10-20 4:42 ` Jason Wang
0 siblings, 2 replies; 6+ messages in thread
From: Dana Rubin @ 2015-10-15 10:54 UTC (permalink / raw)
To: Jason Wang, Dmitry Fleytman
Cc: Leonid Shatz, Dana Rubin, Idan Brown, qemu-devel, Shmulik Ladkani
From: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
Guest OS may issue VMXNET3_CMD_GET_STATS even before device was
activated (for example in linux, after insmod but prior net-dev open).
Accessing shared descriptors prior device activation is illegal as the
VMXNET3State structures have not been fully initialized.
As a result, guest memory gets corrupted and may lead to guest OS
crashes.
Fix, by not filling the stats descriptors if device is inactive.
Reported-by: Leonid Shatz <leonid.shatz@ravellosystems.com>
Signed-off-by: Dana Rubin <dana.rubin@ravellosystems.com>
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
---
hw/net/vmxnet3.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 3c5e10d..5e3a233 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -1289,6 +1289,10 @@ static uint32_t vmxnet3_get_interrupt_config(VMXNET3State *s)
static void vmxnet3_fill_stats(VMXNET3State *s)
{
int i;
+
+ if (!s->device_active)
+ return;
+
for (i = 0; i < s->txq_num; i++) {
cpu_physical_memory_write(s->txq_descr[i].tx_stats_pa,
&s->txq_descr[i].txq_stats,
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] vmxnet3: Do not fill stats if device is inactive
2015-10-15 10:54 [Qemu-devel] [PATCH] vmxnet3: Do not fill stats if device is inactive Dana Rubin
@ 2015-10-18 7:16 ` Dmitry Fleytman
2015-10-20 3:08 ` Jason Wang
2015-10-20 4:42 ` Jason Wang
1 sibling, 1 reply; 6+ messages in thread
From: Dmitry Fleytman @ 2015-10-18 7:16 UTC (permalink / raw)
To: Dana Rubin; +Cc: Leonid Shatz, Jason Wang, Idan Brown, qemu-devel, Dana Rubin
ACK
> On 15 Oct 2015, at 13:54 PM, Dana Rubin <shmulik.ladkani@ravellosystems.com> wrote:
>
> From: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
>
> Guest OS may issue VMXNET3_CMD_GET_STATS even before device was
> activated (for example in linux, after insmod but prior net-dev open).
>
> Accessing shared descriptors prior device activation is illegal as the
> VMXNET3State structures have not been fully initialized.
>
> As a result, guest memory gets corrupted and may lead to guest OS
> crashes.
>
> Fix, by not filling the stats descriptors if device is inactive.
>
> Reported-by: Leonid Shatz <leonid.shatz@ravellosystems.com>
> Signed-off-by: Dana Rubin <dana.rubin@ravellosystems.com>
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
> ---
> hw/net/vmxnet3.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 3c5e10d..5e3a233 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -1289,6 +1289,10 @@ static uint32_t vmxnet3_get_interrupt_config(VMXNET3State *s)
> static void vmxnet3_fill_stats(VMXNET3State *s)
> {
> int i;
> +
> + if (!s->device_active)
> + return;
> +
> for (i = 0; i < s->txq_num; i++) {
> cpu_physical_memory_write(s->txq_descr[i].tx_stats_pa,
> &s->txq_descr[i].txq_stats,
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] vmxnet3: Do not fill stats if device is inactive
2015-10-18 7:16 ` Dmitry Fleytman
@ 2015-10-20 3:08 ` Jason Wang
2015-10-20 7:11 ` Dmitry Fleytman
0 siblings, 1 reply; 6+ messages in thread
From: Jason Wang @ 2015-10-20 3:08 UTC (permalink / raw)
To: Dmitry Fleytman, Dana Rubin
Cc: Leonid Shatz, Idan Brown, qemu-devel, Dana Rubin
On 10/18/2015 03:16 PM, Dmitry Fleytman wrote:
> ACK
Hi Dmitry:
Thanks a lot for the reviewing.
As I want to add your "Acked-by" in the patch, could you pls add a
formal one in the future? (Which can make my life a little bit easier).
>> On 15 Oct 2015, at 13:54 PM, Dana Rubin <shmulik.ladkani@ravellosystems.com> wrote:
>>
>> From: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
>>
>> Guest OS may issue VMXNET3_CMD_GET_STATS even before device was
>> activated (for example in linux, after insmod but prior net-dev open).
>>
>> Accessing shared descriptors prior device activation is illegal as the
>> VMXNET3State structures have not been fully initialized.
>>
>> As a result, guest memory gets corrupted and may lead to guest OS
>> crashes.
>>
>> Fix, by not filling the stats descriptors if device is inactive.
>>
>> Reported-by: Leonid Shatz <leonid.shatz@ravellosystems.com>
>> Signed-off-by: Dana Rubin <dana.rubin@ravellosystems.com>
>> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
>> ---
>> hw/net/vmxnet3.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
>> index 3c5e10d..5e3a233 100644
>> --- a/hw/net/vmxnet3.c
>> +++ b/hw/net/vmxnet3.c
>> @@ -1289,6 +1289,10 @@ static uint32_t vmxnet3_get_interrupt_config(VMXNET3State *s)
>> static void vmxnet3_fill_stats(VMXNET3State *s)
>> {
>> int i;
>> +
>> + if (!s->device_active)
>> + return;
>> +
>> for (i = 0; i < s->txq_num; i++) {
>> cpu_physical_memory_write(s->txq_descr[i].tx_stats_pa,
>> &s->txq_descr[i].txq_stats,
>> --
>> 1.9.1
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] vmxnet3: Do not fill stats if device is inactive
2015-10-20 3:08 ` Jason Wang
@ 2015-10-20 7:11 ` Dmitry Fleytman
2015-10-20 7:18 ` Jason Wang
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Fleytman @ 2015-10-20 7:11 UTC (permalink / raw)
To: Jason Wang; +Cc: Leonid Shatz, Dana Rubin, Idan Brown, Dana Rubin, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1976 bytes --]
Hi Jason,
Sure. No problem.
Acked-by: Dmitry Fleytman <dmitry@daynix.com <mailto:dmitry@daynix.com>>
Dmitry.
> On 20 Oct 2015, at 06:08 AM, Jason Wang <jasowang@redhat.com> wrote:
>
>
>
> On 10/18/2015 03:16 PM, Dmitry Fleytman wrote:
>> ACK
>
> Hi Dmitry:
>
> Thanks a lot for the reviewing.
>
> As I want to add your "Acked-by" in the patch, could you pls add a
> formal one in the future? (Which can make my life a little bit easier).
>
>>> On 15 Oct 2015, at 13:54 PM, Dana Rubin <shmulik.ladkani@ravellosystems.com> wrote:
>>>
>>> From: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
>>>
>>> Guest OS may issue VMXNET3_CMD_GET_STATS even before device was
>>> activated (for example in linux, after insmod but prior net-dev open).
>>>
>>> Accessing shared descriptors prior device activation is illegal as the
>>> VMXNET3State structures have not been fully initialized.
>>>
>>> As a result, guest memory gets corrupted and may lead to guest OS
>>> crashes.
>>>
>>> Fix, by not filling the stats descriptors if device is inactive.
>>>
>>> Reported-by: Leonid Shatz <leonid.shatz@ravellosystems.com>
>>> Signed-off-by: Dana Rubin <dana.rubin@ravellosystems.com>
>>> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
>>> ---
>>> hw/net/vmxnet3.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
>>> index 3c5e10d..5e3a233 100644
>>> --- a/hw/net/vmxnet3.c
>>> +++ b/hw/net/vmxnet3.c
>>> @@ -1289,6 +1289,10 @@ static uint32_t vmxnet3_get_interrupt_config(VMXNET3State *s)
>>> static void vmxnet3_fill_stats(VMXNET3State *s)
>>> {
>>> int i;
>>> +
>>> + if (!s->device_active)
>>> + return;
>>> +
>>> for (i = 0; i < s->txq_num; i++) {
>>> cpu_physical_memory_write(s->txq_descr[i].tx_stats_pa,
>>> &s->txq_descr[i].txq_stats,
>>> --
>>> 1.9.1
>>>
>>
>
[-- Attachment #2: Type: text/html, Size: 3802 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] vmxnet3: Do not fill stats if device is inactive
2015-10-20 7:11 ` Dmitry Fleytman
@ 2015-10-20 7:18 ` Jason Wang
0 siblings, 0 replies; 6+ messages in thread
From: Jason Wang @ 2015-10-20 7:18 UTC (permalink / raw)
To: Dmitry Fleytman
Cc: Leonid Shatz, Dana Rubin, Idan Brown, Dana Rubin, qemu-devel
On 10/20/2015 03:11 PM, Dmitry Fleytman wrote:
> Hi Jason,
>
> Sure. No problem.
>
> Acked-by: Dmitry Fleytman <dmitry@daynix.com <mailto:dmitry@daynix.com>>
>
> Dmitry.
Thanks.
>
>> On 20 Oct 2015, at 06:08 AM, Jason Wang <jasowang@redhat.com
>> <mailto:jasowang@redhat.com>> wrote:
>>
>>
>>
>> On 10/18/2015 03:16 PM, Dmitry Fleytman wrote:
>>> ACK
>>
>> Hi Dmitry:
>>
>> Thanks a lot for the reviewing.
>>
>> As I want to add your "Acked-by" in the patch, could you pls add a
>> formal one in the future? (Which can make my life a little bit easier).
>>
>>>> On 15 Oct 2015, at 13:54 PM, Dana Rubin
>>>> <shmulik.ladkani@ravellosystems.com
>>>> <mailto:shmulik.ladkani@ravellosystems.com>> wrote:
>>>>
>>>> From: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com
>>>> <mailto:shmulik.ladkani@ravellosystems.com>>
>>>>
>>>> Guest OS may issue VMXNET3_CMD_GET_STATS even before device was
>>>> activated (for example in linux, after insmod but prior net-dev open).
>>>>
>>>> Accessing shared descriptors prior device activation is illegal as the
>>>> VMXNET3State structures have not been fully initialized.
>>>>
>>>> As a result, guest memory gets corrupted and may lead to guest OS
>>>> crashes.
>>>>
>>>> Fix, by not filling the stats descriptors if device is inactive.
>>>>
>>>> Reported-by: Leonid Shatz <leonid.shatz@ravellosystems.com
>>>> <mailto:leonid.shatz@ravellosystems.com>>
>>>> Signed-off-by: Dana Rubin <dana.rubin@ravellosystems.com
>>>> <mailto:dana.rubin@ravellosystems.com>>
>>>> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com
>>>> <mailto:shmulik.ladkani@ravellosystems.com>>
>>>> ---
>>>> hw/net/vmxnet3.c | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
>>>> index 3c5e10d..5e3a233 100644
>>>> --- a/hw/net/vmxnet3.c
>>>> +++ b/hw/net/vmxnet3.c
>>>> @@ -1289,6 +1289,10 @@ static uint32_t
>>>> vmxnet3_get_interrupt_config(VMXNET3State *s)
>>>> static void vmxnet3_fill_stats(VMXNET3State *s)
>>>> {
>>>> int i;
>>>> +
>>>> + if (!s->device_active)
>>>> + return;
>>>> +
>>>> for (i = 0; i < s->txq_num; i++) {
>>>> cpu_physical_memory_write(s->txq_descr[i].tx_stats_pa,
>>>> &s->txq_descr[i].txq_stats,
>>>> --
>>>> 1.9.1
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] vmxnet3: Do not fill stats if device is inactive
2015-10-15 10:54 [Qemu-devel] [PATCH] vmxnet3: Do not fill stats if device is inactive Dana Rubin
2015-10-18 7:16 ` Dmitry Fleytman
@ 2015-10-20 4:42 ` Jason Wang
1 sibling, 0 replies; 6+ messages in thread
From: Jason Wang @ 2015-10-20 4:42 UTC (permalink / raw)
To: Dana Rubin, Dmitry Fleytman
Cc: Leonid Shatz, Idan Brown, Dana Rubin, qemu-devel
On 10/15/2015 06:54 PM, Dana Rubin wrote:
> From: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
>
> Guest OS may issue VMXNET3_CMD_GET_STATS even before device was
> activated (for example in linux, after insmod but prior net-dev open).
>
> Accessing shared descriptors prior device activation is illegal as the
> VMXNET3State structures have not been fully initialized.
>
> As a result, guest memory gets corrupted and may lead to guest OS
> crashes.
>
> Fix, by not filling the stats descriptors if device is inactive.
>
> Reported-by: Leonid Shatz <leonid.shatz@ravellosystems.com>
> Signed-off-by: Dana Rubin <dana.rubin@ravellosystems.com>
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
> ---
> hw/net/vmxnet3.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 3c5e10d..5e3a233 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -1289,6 +1289,10 @@ static uint32_t vmxnet3_get_interrupt_config(VMXNET3State *s)
> static void vmxnet3_fill_stats(VMXNET3State *s)
> {
> int i;
> +
> + if (!s->device_active)
> + return;
> +
> for (i = 0; i < s->txq_num; i++) {
> cpu_physical_memory_write(s->txq_descr[i].tx_stats_pa,
> &s->txq_descr[i].txq_stats,
Applied in https://github.com/jasowang/qemu/commits/net
Thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-10-20 7:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-15 10:54 [Qemu-devel] [PATCH] vmxnet3: Do not fill stats if device is inactive Dana Rubin
2015-10-18 7:16 ` Dmitry Fleytman
2015-10-20 3:08 ` Jason Wang
2015-10-20 7:11 ` Dmitry Fleytman
2015-10-20 7:18 ` Jason Wang
2015-10-20 4:42 ` Jason Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).