Re: [PATCH] net: vmxnet3: validate configuration values during activate (CVE-2021-20203)

[Thomas Huth]

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