| Message ID | 20210130131652.954143-1-ppandit@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: vmxnet3: validate configuration values during activate (CVE-2021-20203) | expand |
On 30/01/2021 14.16, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > While activating device in vmxnet3_acticate_device(), it does not > validate guest supplied configuration values against predefined > minimum - maximum limits. This may lead to integer overflow or > OOB access issues. Add checks to avoid it. > > Fixes: CVE-2021-20203 > Buglink: https://bugs.launchpad.net/qemu/+bug/1913873 > Reported-by: Gaoning Pan <pgn@zju.edu.cn> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > hw/net/vmxnet3.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c > index eff299f629..4a910ca971 100644 > --- a/hw/net/vmxnet3.c > +++ b/hw/net/vmxnet3.c > @@ -1420,6 +1420,7 @@ static void vmxnet3_activate_device(VMXNET3State *s) > vmxnet3_setup_rx_filtering(s); > /* Cache fields from shared memory */ > s->mtu = VMXNET3_READ_DRV_SHARED32(d, s->drv_shmem, devRead.misc.mtu); > + assert(VMXNET3_MIN_MTU <= s->mtu && s->mtu < VMXNET3_MAX_MTU); > VMW_CFPRN("MTU is %u", s->mtu); > > s->max_rx_frags = > @@ -1473,6 +1474,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) > /* Read rings memory locations for TX queues */ > pa = VMXNET3_READ_TX_QUEUE_DESCR64(d, qdescr_pa, conf.txRingBasePA); > size = VMXNET3_READ_TX_QUEUE_DESCR32(d, qdescr_pa, conf.txRingSize); > + if (size > VMXNET3_TX_RING_MAX_SIZE) { > + size = VMXNET3_TX_RING_MAX_SIZE; > + } > > vmxnet3_ring_init(d, &s->txq_descr[i].tx_ring, pa, size, > sizeof(struct Vmxnet3_TxDesc), false); > @@ -1483,6 +1487,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) > /* TXC ring */ > pa = VMXNET3_READ_TX_QUEUE_DESCR64(d, qdescr_pa, conf.compRingBasePA); > size = VMXNET3_READ_TX_QUEUE_DESCR32(d, qdescr_pa, conf.compRingSize); > + if (size > VMXNET3_TC_RING_MAX_SIZE) { > + size = VMXNET3_TC_RING_MAX_SIZE; > + } > vmxnet3_ring_init(d, &s->txq_descr[i].comp_ring, pa, size, > sizeof(struct Vmxnet3_TxCompDesc), true); > VMXNET3_RING_DUMP(VMW_CFPRN, "TXC", i, &s->txq_descr[i].comp_ring); > @@ -1524,6 +1531,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) > /* RX rings */ > pa = VMXNET3_READ_RX_QUEUE_DESCR64(d, qd_pa, conf.rxRingBasePA[j]); > size = VMXNET3_READ_RX_QUEUE_DESCR32(d, qd_pa, conf.rxRingSize[j]); > + if (size > VMXNET3_RX_RING_MAX_SIZE) { > + size = VMXNET3_RX_RING_MAX_SIZE; > + } > vmxnet3_ring_init(d, &s->rxq_descr[i].rx_ring[j], pa, size, > sizeof(struct Vmxnet3_RxDesc), false); > VMW_CFPRN("RX queue %d:%d: Base: %" PRIx64 ", Size: %d", > @@ -1533,6 +1543,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) > /* RXC ring */ > pa = VMXNET3_READ_RX_QUEUE_DESCR64(d, qd_pa, conf.compRingBasePA); > size = VMXNET3_READ_RX_QUEUE_DESCR32(d, qd_pa, conf.compRingSize); > + if (size > VMXNET3_RC_RING_MAX_SIZE) { > + size = VMXNET3_RC_RING_MAX_SIZE; > + } > vmxnet3_ring_init(d, &s->rxq_descr[i].comp_ring, pa, size, > sizeof(struct Vmxnet3_RxCompDesc), true); > VMW_CFPRN("RXC queue %d: Base: %" PRIx64 ", Size: %d", i, pa, size); > Ping! According to https://gitlab.com/qemu-project/qemu/-/issues/308#note_705736713 this is still an issue... Patch looks fine to me ... maybe just add some qemu_log_mask(LOG_GUEST_ERROR, ...) statements before correcting the values? Thomas
On Monday, 18 October, 2021, 12:20:55 pm IST, Thomas Huth <thuth@redhat.com> wrote: On 30/01/2021 14.16, P J P wrote: >> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c >> index eff299f629..4a910ca971 100644 >> --- a/hw/net/vmxnet3.c >> +++ b/hw/net/vmxnet3.c >> @@ -1420,6 +1420,7 @@ static void vmxnet3_activate_device(VMXNET3State *s) >> vmxnet3_setup_rx_filtering(s); >> /* Cache fields from shared memory */ >> s->mtu = VMXNET3_READ_DRV_SHARED32(d, s->drv_shmem, devRead.misc.mtu); >> + assert(VMXNET3_MIN_MTU <= s->mtu && s->mtu < VMXNET3_MAX_MTU); >> VMW_CFPRN("MTU is %u", s->mtu); >> >> s->max_rx_frags = >> @@ -1473,6 +1474,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) >> /* Read rings memory locations for TX queues */ >> pa = VMXNET3_READ_TX_QUEUE_DESCR64(d, qdescr_pa, conf.txRingBasePA); >> size = VMXNET3_READ_TX_QUEUE_DESCR32(d, qdescr_pa, conf.txRingSize); >> + if (size > VMXNET3_TX_RING_MAX_SIZE) { >> + size = VMXNET3_TX_RING_MAX_SIZE; >> + } >> >> vmxnet3_ring_init(d, &s->txq_descr[i].tx_ring, pa, size, >> sizeof(struct Vmxnet3_TxDesc), false); >> @@ -1483,6 +1487,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) >> /* TXC ring */ >> pa = VMXNET3_READ_TX_QUEUE_DESCR64(d, qdescr_pa, conf.compRingBasePA); >> size = VMXNET3_READ_TX_QUEUE_DESCR32(d, qdescr_pa, conf.compRingSize); >> + if (size > VMXNET3_TC_RING_MAX_SIZE) { >> + size = VMXNET3_TC_RING_MAX_SIZE; >> + } >> vmxnet3_ring_init(d, &s->txq_descr[i].comp_ring, pa, size, >> sizeof(struct Vmxnet3_TxCompDesc), true); >> VMXNET3_RING_DUMP(VMW_CFPRN, "TXC", i, &s->txq_descr[i].comp_ring); >> @@ -1524,6 +1531,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) >> /* RX rings */ >> pa = VMXNET3_READ_RX_QUEUE_DESCR64(d, qd_pa, conf.rxRingBasePA[j]); >> size = VMXNET3_READ_RX_QUEUE_DESCR32(d, qd_pa, conf.rxRingSize[j]); >> + if (size > VMXNET3_RX_RING_MAX_SIZE) { >> + size = VMXNET3_RX_RING_MAX_SIZE; >> + } >> vmxnet3_ring_init(d, &s->rxq_descr[i].rx_ring[j], pa, size, >> sizeof(struct Vmxnet3_RxDesc), false); >> VMW_CFPRN("RX queue %d:%d: Base: %" PRIx64 ", Size: %d", >> @@ -1533,6 +1543,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) >> /* RXC ring */ >> pa = VMXNET3_READ_RX_QUEUE_DESCR64(d, qd_pa, conf.compRingBasePA); >> size = VMXNET3_READ_RX_QUEUE_DESCR32(d, qd_pa, conf.compRingSize); >> + if (size > VMXNET3_RC_RING_MAX_SIZE) { >> + size = VMXNET3_RC_RING_MAX_SIZE; >> + } >> vmxnet3_ring_init(d, &s->rxq_descr[i].comp_ring, pa, size, >> sizeof(struct Vmxnet3_RxCompDesc), true); >> VMW_CFPRN("RXC queue %d: Base: %" PRIx64 ", Size: %d", i, pa, size); >> >> >Ping! > >According to >https://gitlab.com/qemu-project/qemu/-/issues/308#note_705736713 this is >still an issue... > >Patch looks fine to me ... maybe just add some >qemu_log_mask(LOG_GUEST_ERROR, ...) statements before correcting the values? * Oops! Not sure how I missed it, thought it was pulled upstream. Will send a revised patch. Thank you. --- - P J P
ping? On 10/18/21 11:09, P J P wrote: > On Monday, 18 October, 2021, 12:20:55 pm IST, Thomas Huth <thuth@redhat.com> wrote: > On 30/01/2021 14.16, P J P wrote: >>> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c >>> index eff299f629..4a910ca971 100644 >>> --- a/hw/net/vmxnet3.c >>> +++ b/hw/net/vmxnet3.c >>> @@ -1420,6 +1420,7 @@ static void vmxnet3_activate_device(VMXNET3State *s) >>> vmxnet3_setup_rx_filtering(s); >>> /* Cache fields from shared memory */ >>> s->mtu = VMXNET3_READ_DRV_SHARED32(d, s->drv_shmem, devRead.misc.mtu); >>> + assert(VMXNET3_MIN_MTU <= s->mtu && s->mtu < VMXNET3_MAX_MTU); >>> VMW_CFPRN("MTU is %u", s->mtu); >>> >>> s->max_rx_frags = >>> @@ -1473,6 +1474,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) >>> /* Read rings memory locations for TX queues */ >>> pa = VMXNET3_READ_TX_QUEUE_DESCR64(d, qdescr_pa, conf.txRingBasePA); >>> size = VMXNET3_READ_TX_QUEUE_DESCR32(d, qdescr_pa, conf.txRingSize); >>> + if (size > VMXNET3_TX_RING_MAX_SIZE) { >>> + size = VMXNET3_TX_RING_MAX_SIZE; >>> + } >>> >>> vmxnet3_ring_init(d, &s->txq_descr[i].tx_ring, pa, size, >>> sizeof(struct Vmxnet3_TxDesc), false); >>> @@ -1483,6 +1487,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) >>> /* TXC ring */ >>> pa = VMXNET3_READ_TX_QUEUE_DESCR64(d, qdescr_pa, conf.compRingBasePA); >>> size = VMXNET3_READ_TX_QUEUE_DESCR32(d, qdescr_pa, conf.compRingSize); >>> + if (size > VMXNET3_TC_RING_MAX_SIZE) { >>> + size = VMXNET3_TC_RING_MAX_SIZE; >>> + } >>> vmxnet3_ring_init(d, &s->txq_descr[i].comp_ring, pa, size, >>> sizeof(struct Vmxnet3_TxCompDesc), true); >>> VMXNET3_RING_DUMP(VMW_CFPRN, "TXC", i, &s->txq_descr[i].comp_ring); >>> @@ -1524,6 +1531,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) >>> /* RX rings */ >>> pa = VMXNET3_READ_RX_QUEUE_DESCR64(d, qd_pa, conf.rxRingBasePA[j]); >>> size = VMXNET3_READ_RX_QUEUE_DESCR32(d, qd_pa, conf.rxRingSize[j]); >>> + if (size > VMXNET3_RX_RING_MAX_SIZE) { >>> + size = VMXNET3_RX_RING_MAX_SIZE; >>> + } >>> vmxnet3_ring_init(d, &s->rxq_descr[i].rx_ring[j], pa, size, >>> sizeof(struct Vmxnet3_RxDesc), false); >>> VMW_CFPRN("RX queue %d:%d: Base: %" PRIx64 ", Size: %d", >>> @@ -1533,6 +1543,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) >>> /* RXC ring */ >>> pa = VMXNET3_READ_RX_QUEUE_DESCR64(d, qd_pa, conf.compRingBasePA); >>> size = VMXNET3_READ_RX_QUEUE_DESCR32(d, qd_pa, conf.compRingSize); >>> + if (size > VMXNET3_RC_RING_MAX_SIZE) { >>> + size = VMXNET3_RC_RING_MAX_SIZE; >>> + } >>> vmxnet3_ring_init(d, &s->rxq_descr[i].comp_ring, pa, size, >>> sizeof(struct Vmxnet3_RxCompDesc), true); >>> VMW_CFPRN("RXC queue %d: Base: %" PRIx64 ", Size: %d", i, pa, size); >>> >>> >> Ping! >> >> According to >> https://gitlab.com/qemu-project/qemu/-/issues/308#note_705736713 this is >> still an issue... >> >> Patch looks fine to me ... maybe just add some >> qemu_log_mask(LOG_GUEST_ERROR, ...) statements before correcting the values? > > > * Oops! Not sure how I missed it, thought it was pulled upstream. > Will send a revised patch. > > > Thank you. > --- > - P J P >
在 2021/11/18 下午8:32, Philippe Mathieu-Daudé 写道: > ping? Applied. Thanks > > On 10/18/21 11:09, P J P wrote: >> On Monday, 18 October, 2021, 12:20:55 pm IST, Thomas Huth <thuth@redhat.com> wrote: >> On 30/01/2021 14.16, P J P wrote: >>>> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c >>>> index eff299f629..4a910ca971 100644 >>>> --- a/hw/net/vmxnet3.c >>>> +++ b/hw/net/vmxnet3.c >>>> @@ -1420,6 +1420,7 @@ static void vmxnet3_activate_device(VMXNET3State *s) >>>> vmxnet3_setup_rx_filtering(s); >>>> /* Cache fields from shared memory */ >>>> s->mtu = VMXNET3_READ_DRV_SHARED32(d, s->drv_shmem, devRead.misc.mtu); >>>> + assert(VMXNET3_MIN_MTU <= s->mtu && s->mtu < VMXNET3_MAX_MTU); >>>> VMW_CFPRN("MTU is %u", s->mtu); >>>> >>>> s->max_rx_frags = >>>> @@ -1473,6 +1474,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) >>>> /* Read rings memory locations for TX queues */ >>>> pa = VMXNET3_READ_TX_QUEUE_DESCR64(d, qdescr_pa, conf.txRingBasePA); >>>> size = VMXNET3_READ_TX_QUEUE_DESCR32(d, qdescr_pa, conf.txRingSize); >>>> + if (size > VMXNET3_TX_RING_MAX_SIZE) { >>>> + size = VMXNET3_TX_RING_MAX_SIZE; >>>> + } >>>> >>>> vmxnet3_ring_init(d, &s->txq_descr[i].tx_ring, pa, size, >>>> sizeof(struct Vmxnet3_TxDesc), false); >>>> @@ -1483,6 +1487,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) >>>> /* TXC ring */ >>>> pa = VMXNET3_READ_TX_QUEUE_DESCR64(d, qdescr_pa, conf.compRingBasePA); >>>> size = VMXNET3_READ_TX_QUEUE_DESCR32(d, qdescr_pa, conf.compRingSize); >>>> + if (size > VMXNET3_TC_RING_MAX_SIZE) { >>>> + size = VMXNET3_TC_RING_MAX_SIZE; >>>> + } >>>> vmxnet3_ring_init(d, &s->txq_descr[i].comp_ring, pa, size, >>>> sizeof(struct Vmxnet3_TxCompDesc), true); >>>> VMXNET3_RING_DUMP(VMW_CFPRN, "TXC", i, &s->txq_descr[i].comp_ring); >>>> @@ -1524,6 +1531,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) >>>> /* RX rings */ >>>> pa = VMXNET3_READ_RX_QUEUE_DESCR64(d, qd_pa, conf.rxRingBasePA[j]); >>>> size = VMXNET3_READ_RX_QUEUE_DESCR32(d, qd_pa, conf.rxRingSize[j]); >>>> + if (size > VMXNET3_RX_RING_MAX_SIZE) { >>>> + size = VMXNET3_RX_RING_MAX_SIZE; >>>> + } >>>> vmxnet3_ring_init(d, &s->rxq_descr[i].rx_ring[j], pa, size, >>>> sizeof(struct Vmxnet3_RxDesc), false); >>>> VMW_CFPRN("RX queue %d:%d: Base: %" PRIx64 ", Size: %d", >>>> @@ -1533,6 +1543,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) >>>> /* RXC ring */ >>>> pa = VMXNET3_READ_RX_QUEUE_DESCR64(d, qd_pa, conf.compRingBasePA); >>>> size = VMXNET3_READ_RX_QUEUE_DESCR32(d, qd_pa, conf.compRingSize); >>>> + if (size > VMXNET3_RC_RING_MAX_SIZE) { >>>> + size = VMXNET3_RC_RING_MAX_SIZE; >>>> + } >>>> vmxnet3_ring_init(d, &s->rxq_descr[i].comp_ring, pa, size, >>>> sizeof(struct Vmxnet3_RxCompDesc), true); >>>> VMW_CFPRN("RXC queue %d: Base: %" PRIx64 ", Size: %d", i, pa, size); >>>> >>>> >>> Ping! >>> >>> According to >>> https://gitlab.com/qemu-project/qemu/-/issues/308#note_705736713 this is >>> still an issue... >>> >>> Patch looks fine to me ... maybe just add some >>> qemu_log_mask(LOG_GUEST_ERROR, ...) statements before correcting the values? >> >> * Oops! Not sure how I missed it, thought it was pulled upstream. >> Will send a revised patch. >> >> >> Thank you. >> --- >> - P J P >>
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index eff299f629..4a910ca971 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -1420,6 +1420,7 @@ static void vmxnet3_activate_device(VMXNET3State *s) vmxnet3_setup_rx_filtering(s); /* Cache fields from shared memory */ s->mtu = VMXNET3_READ_DRV_SHARED32(d, s->drv_shmem, devRead.misc.mtu); + assert(VMXNET3_MIN_MTU <= s->mtu && s->mtu < VMXNET3_MAX_MTU); VMW_CFPRN("MTU is %u", s->mtu); s->max_rx_frags = @@ -1473,6 +1474,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) /* Read rings memory locations for TX queues */ pa = VMXNET3_READ_TX_QUEUE_DESCR64(d, qdescr_pa, conf.txRingBasePA); size = VMXNET3_READ_TX_QUEUE_DESCR32(d, qdescr_pa, conf.txRingSize); + if (size > VMXNET3_TX_RING_MAX_SIZE) { + size = VMXNET3_TX_RING_MAX_SIZE; + } vmxnet3_ring_init(d, &s->txq_descr[i].tx_ring, pa, size, sizeof(struct Vmxnet3_TxDesc), false); @@ -1483,6 +1487,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) /* TXC ring */ pa = VMXNET3_READ_TX_QUEUE_DESCR64(d, qdescr_pa, conf.compRingBasePA); size = VMXNET3_READ_TX_QUEUE_DESCR32(d, qdescr_pa, conf.compRingSize); + if (size > VMXNET3_TC_RING_MAX_SIZE) { + size = VMXNET3_TC_RING_MAX_SIZE; + } vmxnet3_ring_init(d, &s->txq_descr[i].comp_ring, pa, size, sizeof(struct Vmxnet3_TxCompDesc), true); VMXNET3_RING_DUMP(VMW_CFPRN, "TXC", i, &s->txq_descr[i].comp_ring); @@ -1524,6 +1531,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) /* RX rings */ pa = VMXNET3_READ_RX_QUEUE_DESCR64(d, qd_pa, conf.rxRingBasePA[j]); size = VMXNET3_READ_RX_QUEUE_DESCR32(d, qd_pa, conf.rxRingSize[j]); + if (size > VMXNET3_RX_RING_MAX_SIZE) { + size = VMXNET3_RX_RING_MAX_SIZE; + } vmxnet3_ring_init(d, &s->rxq_descr[i].rx_ring[j], pa, size, sizeof(struct Vmxnet3_RxDesc), false); VMW_CFPRN("RX queue %d:%d: Base: %" PRIx64 ", Size: %d", @@ -1533,6 +1543,9 @@ static void vmxnet3_activate_device(VMXNET3State *s) /* RXC ring */ pa = VMXNET3_READ_RX_QUEUE_DESCR64(d, qd_pa, conf.compRingBasePA); size = VMXNET3_READ_RX_QUEUE_DESCR32(d, qd_pa, conf.compRingSize); + if (size > VMXNET3_RC_RING_MAX_SIZE) { + size = VMXNET3_RC_RING_MAX_SIZE; + } vmxnet3_ring_init(d, &s->rxq_descr[i].comp_ring, pa, size, sizeof(struct Vmxnet3_RxCompDesc), true); VMW_CFPRN("RXC queue %d: Base: %" PRIx64 ", Size: %d", i, pa, size);