| Message ID | 20230817125600.1440195-1-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/net/vmxnet3: Fix guest-triggerable assert() | expand |
Hi Thomas, On 17/8/23 14:56, Thomas Huth wrote: > The assert() that checks for valid MTU sizes can be triggered by > the guest (e.g. with the reproducer code from the bug ticket > https://gitlab.com/qemu-project/qemu/-/issues/517 ). Let's avoid > this problem by simply logging the error and refusing to activate > the device instead. > > Fixes: d05dcd94ae ("net: vmxnet3: validate configuration values during activate") > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > hw/net/vmxnet3.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c > index 5dfacb1098..6674122a7e 100644 > --- a/hw/net/vmxnet3.c > +++ b/hw/net/vmxnet3.c > @@ -1439,7 +1439,10 @@ 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); > + if (s->mtu < VMXNET3_MIN_MTU || s->mtu > VMXNET3_MAX_MTU) { > + qemu_log_mask(LOG_GUEST_ERROR, "vmxnet3: Bad MTU size: %d\n", s->mtu); mtu is uint32_t, otherwise: Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > + return; > + } > VMW_CFPRN("MTU is %u", s->mtu); > > s->max_rx_frags =
17.08.2023 15:56, Thomas Huth wrote: > The assert() that checks for valid MTU sizes can be triggered by > the guest (e.g. with the reproducer code from the bug ticket > https://gitlab.com/qemu-project/qemu/-/issues/517 ). Let's avoid > this problem by simply logging the error and refusing to activate > the device instead. I'm applying this to trivial-patches tree with some hesitation :) Thanks, /mjt > Fixes: d05dcd94ae ("net: vmxnet3: validate configuration values during activate") > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > hw/net/vmxnet3.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c > index 5dfacb1098..6674122a7e 100644 > --- a/hw/net/vmxnet3.c > +++ b/hw/net/vmxnet3.c > @@ -1439,7 +1439,10 @@ 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); > + if (s->mtu < VMXNET3_MIN_MTU || s->mtu > VMXNET3_MAX_MTU) { > + qemu_log_mask(LOG_GUEST_ERROR, "vmxnet3: Bad MTU size: %d\n", s->mtu); > + return; > + } > VMW_CFPRN("MTU is %u", s->mtu); > > s->max_rx_frags =
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index 5dfacb1098..6674122a7e 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -1439,7 +1439,10 @@ 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); + if (s->mtu < VMXNET3_MIN_MTU || s->mtu > VMXNET3_MAX_MTU) { + qemu_log_mask(LOG_GUEST_ERROR, "vmxnet3: Bad MTU size: %d\n", s->mtu); + return; + } VMW_CFPRN("MTU is %u", s->mtu); s->max_rx_frags =
The assert() that checks for valid MTU sizes can be triggered by the guest (e.g. with the reproducer code from the bug ticket https://gitlab.com/qemu-project/qemu/-/issues/517 ). Let's avoid this problem by simply logging the error and refusing to activate the device instead. Fixes: d05dcd94ae ("net: vmxnet3: validate configuration values during activate") Signed-off-by: Thomas Huth <thuth@redhat.com> --- hw/net/vmxnet3.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)