| Message ID | CAAKa2jm_4noz=rVc-vW6-4q-HsJtLoZXUZJfoHprjpLmYgb1EA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [qemu] hw/net/vmxnet3: Remove g_assert_not_reached() when VMXNET3_REG_ICR is written | expand |
在 2021/6/23 上午10:26, Qiang Liu 写道: > From: cyruscyliu <cyruscyliu@gmail.com> > > A malicious guest user can write VMXNET3_REG_ICR to crash QEMU. This > patch remove the g_aasert_not_reached() there and make the access pass. > > Fixes: 786fd2b0f87 ("VMXNET3 device implementation") > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/309 > Buglink: https://bugs.launchpad.net/qemu/+bug/1913923 > > Signed-off-by: Qiang Liu <cyruscyliu@gmail.com> Do we need to warn about the unimplemented register? Thanks > --- > hw/net/vmxnet3.c | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c > index eff299f629..a388918479 100644 > --- a/hw/net/vmxnet3.c > +++ b/hw/net/vmxnet3.c > @@ -1786,13 +1786,6 @@ vmxnet3_io_bar1_write(void *opaque, > vmxnet3_set_variable_mac(s, val, s->temp_mac); > break; > > - /* Interrupt Cause Register */ > - case VMXNET3_REG_ICR: > - VMW_CBPRN("Write BAR1 [VMXNET3_REG_ICR] = %" PRIx64 ", size %d", > - val, size); > - g_assert_not_reached(); > - break; > - > /* Event Cause Register */ > case VMXNET3_REG_ECR: > VMW_CBPRN("Write BAR1 [VMXNET3_REG_ECR] = %" PRIx64 ", size %d", > -- > 2.30.2 >
Hi, On Wed, Jun 23, 2021 at 11:23 AM Jason Wang <jasowang@redhat.com> wrote: > > > 在 2021/6/23 上午10:26, Qiang Liu 写道: > > From: cyruscyliu <cyruscyliu@gmail.com> > > > > A malicious guest user can write VMXNET3_REG_ICR to crash QEMU. This > > patch remove the g_aasert_not_reached() there and make the access pass. > > > > Fixes: 786fd2b0f87 ("VMXNET3 device implementation") > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/309 > > Buglink: https://bugs.launchpad.net/qemu/+bug/1913923 > > > > Signed-off-by: Qiang Liu <cyruscyliu@gmail.com> > > > Do we need to warn about the unimplemented register? If we remove the case branch, it will go to the default branch which can warn users if VMXNET_DEBUG_CB_ENABLED is defined, so there is no need to warn this unimplemented register. Am I right? ``` vmxnet3_io_bar1_write(... default: VMW_CBPRN("Unknown Write to BAR1 [%" PRIx64 "] = %" PRIx64 ", size %d", addr, val, size); break; ``` > Thanks > > > > --- > > hw/net/vmxnet3.c | 7 ------- > > 1 file changed, 7 deletions(-) > > > > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c > > index eff299f629..a388918479 100644 > > --- a/hw/net/vmxnet3.c > > +++ b/hw/net/vmxnet3.c > > @@ -1786,13 +1786,6 @@ vmxnet3_io_bar1_write(void *opaque, > > vmxnet3_set_variable_mac(s, val, s->temp_mac); > > break; > > > > - /* Interrupt Cause Register */ > > - case VMXNET3_REG_ICR: > > - VMW_CBPRN("Write BAR1 [VMXNET3_REG_ICR] = %" PRIx64 ", size %d", > > - val, size); > > - g_assert_not_reached(); > > - break; > > - > > /* Event Cause Register */ > > case VMXNET3_REG_ECR: > > VMW_CBPRN("Write BAR1 [VMXNET3_REG_ECR] = %" PRIx64 ", size %d", > > -- > > 2.30.2 > > >
在 2021/6/25 上午10:19, Qiang Liu 写道: > Hi, > On Wed, Jun 23, 2021 at 11:23 AM Jason Wang <jasowang@redhat.com> wrote: >> >> 在 2021/6/23 上午10:26, Qiang Liu 写道: >>> From: cyruscyliu <cyruscyliu@gmail.com> >>> >>> A malicious guest user can write VMXNET3_REG_ICR to crash QEMU. This >>> patch remove the g_aasert_not_reached() there and make the access pass. >>> >>> Fixes: 786fd2b0f87 ("VMXNET3 device implementation") >>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/309 >>> Buglink: https://bugs.launchpad.net/qemu/+bug/1913923 >>> >>> Signed-off-by: Qiang Liu <cyruscyliu@gmail.com> >> >> Do we need to warn about the unimplemented register? > If we remove the case branch, it will go to the default branch which > can warn users if > VMXNET_DEBUG_CB_ENABLED is defined, so there is no need to warn this > unimplemented register. Am I right? > ``` > vmxnet3_io_bar1_write(... > default: > VMW_CBPRN("Unknown Write to BAR1 [%" PRIx64 "] = %" PRIx64 ", size %d", > addr, val, size); > break; > ``` Right. I've queued this patch. Thanks >> Thanks >> >> >>> --- >>> hw/net/vmxnet3.c | 7 ------- >>> 1 file changed, 7 deletions(-) >>> >>> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c >>> index eff299f629..a388918479 100644 >>> --- a/hw/net/vmxnet3.c >>> +++ b/hw/net/vmxnet3.c >>> @@ -1786,13 +1786,6 @@ vmxnet3_io_bar1_write(void *opaque, >>> vmxnet3_set_variable_mac(s, val, s->temp_mac); >>> break; >>> >>> - /* Interrupt Cause Register */ >>> - case VMXNET3_REG_ICR: >>> - VMW_CBPRN("Write BAR1 [VMXNET3_REG_ICR] = %" PRIx64 ", size %d", >>> - val, size); >>> - g_assert_not_reached(); >>> - break; >>> - >>> /* Event Cause Register */ >>> case VMXNET3_REG_ECR: >>> VMW_CBPRN("Write BAR1 [VMXNET3_REG_ECR] = %" PRIx64 ", size %d", >>> -- >>> 2.30.2 >>>
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index eff299f629..a388918479 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -1786,13 +1786,6 @@ vmxnet3_io_bar1_write(void *opaque, vmxnet3_set_variable_mac(s, val, s->temp_mac); break; - /* Interrupt Cause Register */ - case VMXNET3_REG_ICR: - VMW_CBPRN("Write BAR1 [VMXNET3_REG_ICR] = %" PRIx64 ", size %d", - val, size); - g_assert_not_reached(); - break; - /* Event Cause Register */ case VMXNET3_REG_ECR: VMW_CBPRN("Write BAR1 [VMXNET3_REG_ECR] = %" PRIx64 ", size %d",