On 2020/5/25 下午8:23, Philippe Mathieu-Daudé wrote:
> libFuzzer found using 'qemu-system-i386 -M q35':
>
> qemu: hardware error: e1000e: PSRCTL.BSIZE0 cannot be zero
> CPU #0:
> EAX=00000000 EBX=00000000 ECX=00000000 EDX=00000663
> ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000
> EIP=0000fff0 EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0
> ES =0000 00000000 0000ffff 00009300
> CS =f000 ffff0000 0000ffff 00009b00
> SS =0000 00000000 0000ffff 00009300
> DS =0000 00000000 0000ffff 00009300
> FS =0000 00000000 0000ffff 00009300
> GS =0000 00000000 0000ffff 00009300
> LDT=0000 00000000 0000ffff 00008200
> TR =0000 00000000 0000ffff 00008b00
> GDT= 00000000 0000ffff
> IDT= 00000000 0000ffff
> CR0=60000010 CR2=00000000 CR3=00000000 CR4=00000000
> DR0=00000000 DR1=00000000 DR2=00000000 DR3=00000000
> DR6=ffff0ff0 DR7=00000400
> EFER=0000000000000000
> FCW=037f FSW=0000 [ST=0] FTW=00 MXCSR=00001f80
> FPR0=0000000000000000 0000 FPR1=0000000000000000 0000
> FPR2=0000000000000000 0000 FPR3=0000000000000000 0000
> FPR4=0000000000000000 0000 FPR5=0000000000000000 0000
> FPR6=0000000000000000 0000 FPR7=0000000000000000 0000
> XMM00=00000000000000000000000000000000 XMM01=00000000000000000000000000000000
> XMM02=00000000000000000000000000000000 XMM03=00000000000000000000000000000000
> XMM04=00000000000000000000000000000000 XMM05=00000000000000000000000000000000
> XMM06=00000000000000000000000000000000 XMM07=00000000000000000000000000000000
> ==1988== ERROR: libFuzzer: deadly signal
> #6 0x7fae4d3ea894 in __GI_abort (/lib64/libc.so.6+0x22894)
> #7 0x563f4cc59a1d in hw_error (qemu-fuzz-i386+0xe8ca1d)
> #8 0x563f4d7c93f2 in e1000e_set_psrctl (qemu-fuzz-i386+0x19fc3f2)
> #9 0x563f4d7b798f in e1000e_core_write (qemu-fuzz-i386+0x19ea98f)
> #10 0x563f4d7afc46 in e1000e_mmio_write (qemu-fuzz-i386+0x19e2c46)
> #11 0x563f4cc9a0a7 in memory_region_write_accessor (qemu-fuzz-i386+0xecd0a7)
> #12 0x563f4cc99c13 in access_with_adjusted_size (qemu-fuzz-i386+0xeccc13)
> #13 0x563f4cc987b4 in memory_region_dispatch_write (qemu-fuzz-i386+0xecb7b4)
>
> It simply sent the following 2 I/O command to the e1000e
> PCI BAR #2 I/O region:
>
> writew 0x0100 0x0c00 # RCTL = E1000_RCTL_DTYP_MASK
> writeb 0x2170 0x00 # PSRCTL = 0
>
> 2813 static void
> 2814 e1000e_set_psrctl(E1000ECore *core, int index, uint32_t val)
> 2815 {
> 2816 if (core->mac[RCTL] & E1000_RCTL_DTYP_MASK) {
> 2817
> 2818 if ((val & E1000_PSRCTL_BSIZE0_MASK) == 0) {
> 2819 hw_error("e1000e: PSRCTL.BSIZE0 cannot be zero");
> 2820 }
>
> Instead of calling hw_error() which abort the process (it is
> meant for CPU fatal error condition, not for device logging),
> log the invalid request with qemu_log_mask(LOG_GUEST_ERROR)
> and return ignoring the request.
Maybe more understandable adding a comma after "return":
"... and return, ignoring the request."
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> RFC because there might be something clever than returning...
> Probably worth a Cc:qemu-stable@ tag
I can't think of a better solution here. And the code looks safe if we
accept zero value.
So I applied this patch.
Thanks. Can you append the "Cc:qemu-stable@..." tag please?
Thanks
> ---
> hw/net/e1000e_core.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
> index d5676871fa..bcd186cac5 100644
> --- a/hw/net/e1000e_core.c
> +++ b/hw/net/e1000e_core.c
> @@ -34,9 +34,9 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/log.h"
> #include "net/net.h"
> #include "net/tap.h"
> -#include "hw/hw.h"
> #include "hw/pci/msi.h"
> #include "hw/pci/msix.h"
> #include "sysemu/runstate.h"
> @@ -2816,11 +2816,15 @@ e1000e_set_psrctl(E1000ECore *core, int index, uint32_t val)
> if (core->mac[RCTL] & E1000_RCTL_DTYP_MASK) {
>
> if ((val & E1000_PSRCTL_BSIZE0_MASK) == 0) {
> - hw_error("e1000e: PSRCTL.BSIZE0 cannot be zero");
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "e1000e: PSRCTL.BSIZE0 cannot be zero");
> + return;
> }
>
> if ((val & E1000_PSRCTL_BSIZE1_MASK) == 0) {
> - hw_error("e1000e: PSRCTL.BSIZE1 cannot be zero");
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "e1000e: PSRCTL.BSIZE1 cannot be zero");
> + return;
> }
> }
>