* [PATCH v2] hw/net/vmxnet3: allow VMXNET3_MAX_MTU itself as a value
@ 2022-08-25 9:29 Fiona Ebner
2022-12-14 9:43 ` Fiona Ebner
0 siblings, 1 reply; 3+ messages in thread
From: Fiona Ebner @ 2022-08-25 9:29 UTC (permalink / raw)
To: qemu-devel; +Cc: dmitry.fleytman, jasowang, pj.pandit
Currently, VMXNET3_MAX_MTU itself (being 9000) is not considered a
valid value for the MTU, but a guest running ESXi 7.0 might try to
set it and fail the assert [0].
In the Linux kernel, dev->max_mtu itself is a valid value for the MTU
and for the vmxnet3 driver it's 9000, so a guest running Linux will
also fail the assert when trying to set an MTU of 9000.
VMXNET3_MAX_MTU and s->mtu don't seem to be used in relation to buffer
allocations/accesses, so allowing the upper limit itself as a value
should be fine.
[0]: https://forum.proxmox.com/threads/114011/
Fixes: d05dcd94ae ("net: vmxnet3: validate configuration values during activate (CVE-2021-20203)")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Feel free to adapt the commit message as you see fit.
v1 -> v2:
* Add commit message with some rationale.
hw/net/vmxnet3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 0b7acf7f89..a2037583bf 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -1441,7 +1441,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);
+ assert(VMXNET3_MIN_MTU <= s->mtu && s->mtu <= VMXNET3_MAX_MTU);
VMW_CFPRN("MTU is %u", s->mtu);
s->max_rx_frags =
--
2.30.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] hw/net/vmxnet3: allow VMXNET3_MAX_MTU itself as a value
2022-08-25 9:29 [PATCH v2] hw/net/vmxnet3: allow VMXNET3_MAX_MTU itself as a value Fiona Ebner
@ 2022-12-14 9:43 ` Fiona Ebner
2022-12-16 7:38 ` Jason Wang
0 siblings, 1 reply; 3+ messages in thread
From: Fiona Ebner @ 2022-12-14 9:43 UTC (permalink / raw)
To: qemu-devel; +Cc: dmitry.fleytman, jasowang, pj.pandit
Am 25.08.22 um 11:29 schrieb Fiona Ebner:
> Currently, VMXNET3_MAX_MTU itself (being 9000) is not considered a
> valid value for the MTU, but a guest running ESXi 7.0 might try to
> set it and fail the assert [0].
>
> In the Linux kernel, dev->max_mtu itself is a valid value for the MTU
> and for the vmxnet3 driver it's 9000, so a guest running Linux will
> also fail the assert when trying to set an MTU of 9000.
>
> VMXNET3_MAX_MTU and s->mtu don't seem to be used in relation to buffer
> allocations/accesses, so allowing the upper limit itself as a value
> should be fine.
>
> [0]: https://forum.proxmox.com/threads/114011/
>
> Fixes: d05dcd94ae ("net: vmxnet3: validate configuration values during activate (CVE-2021-20203)")
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> Feel free to adapt the commit message as you see fit.
>
> v1 -> v2:
> * Add commit message with some rationale.
>
> hw/net/vmxnet3.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 0b7acf7f89..a2037583bf 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -1441,7 +1441,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);
> + assert(VMXNET3_MIN_MTU <= s->mtu && s->mtu <= VMXNET3_MAX_MTU);
> VMW_CFPRN("MTU is %u", s->mtu);
>
> s->max_rx_frags =
Ping
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] hw/net/vmxnet3: allow VMXNET3_MAX_MTU itself as a value
2022-12-14 9:43 ` Fiona Ebner
@ 2022-12-16 7:38 ` Jason Wang
0 siblings, 0 replies; 3+ messages in thread
From: Jason Wang @ 2022-12-16 7:38 UTC (permalink / raw)
To: Fiona Ebner; +Cc: qemu-devel, dmitry.fleytman, pj.pandit
On Wed, Dec 14, 2022 at 5:43 PM Fiona Ebner <f.ebner@proxmox.com> wrote:
>
> Am 25.08.22 um 11:29 schrieb Fiona Ebner:
> > Currently, VMXNET3_MAX_MTU itself (being 9000) is not considered a
> > valid value for the MTU, but a guest running ESXi 7.0 might try to
> > set it and fail the assert [0].
> >
> > In the Linux kernel, dev->max_mtu itself is a valid value for the MTU
> > and for the vmxnet3 driver it's 9000, so a guest running Linux will
> > also fail the assert when trying to set an MTU of 9000.
> >
> > VMXNET3_MAX_MTU and s->mtu don't seem to be used in relation to buffer
> > allocations/accesses, so allowing the upper limit itself as a value
> > should be fine.
> >
> > [0]: https://forum.proxmox.com/threads/114011/
> >
> > Fixes: d05dcd94ae ("net: vmxnet3: validate configuration values during activate (CVE-2021-20203)")
> > Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> > ---
> >
> > Feel free to adapt the commit message as you see fit.
> >
> > v1 -> v2:
> > * Add commit message with some rationale.
> >
> > hw/net/vmxnet3.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> > index 0b7acf7f89..a2037583bf 100644
> > --- a/hw/net/vmxnet3.c
> > +++ b/hw/net/vmxnet3.c
> > @@ -1441,7 +1441,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);
> > + assert(VMXNET3_MIN_MTU <= s->mtu && s->mtu <= VMXNET3_MAX_MTU);
> > VMW_CFPRN("MTU is %u", s->mtu);
> >
> > s->max_rx_frags =
>
> Ping
I've queued this patch.
Thanks
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-12-16 7:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-25 9:29 [PATCH v2] hw/net/vmxnet3: allow VMXNET3_MAX_MTU itself as a value Fiona Ebner
2022-12-14 9:43 ` Fiona Ebner
2022-12-16 7:38 ` 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).