| Message ID | 1444906470-21216-1-git-send-email-dana.rubin@ravellosystems.com |
|---|---|
| State | New |
| Headers | show |
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 >
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 >> >
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
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 >>> >> >
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 >>>> >>> >> >
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,