From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: Jason Wang <jasowang@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Dmitry Fleytman <dmitry.fleytman@gmail.com>,
"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
"Michael S . Tsirkin" <mst@redhat.com>
Subject: Re: [RFC PATCH] hw/net/e1000e: Do not abort() on invalid PSRCTL register value
Date: Fri, 29 May 2020 09:17:54 +0200 [thread overview]
Message-ID: <CAAdtpL5=-e_MwpK0n3X0TeT_XrYOLHfA8gqTrx0WS7yBRqO6MQ@mail.gmail.com> (raw)
In-Reply-To: <2623aab1-355a-a5dc-3fc1-e22f795b98a0@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 4945 bytes --]
Le ven. 29 mai 2020 08:36, Jason Wang <jasowang@redhat.com> a écrit :
>
> 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;
> > }
> > }
> >
>
>
[-- Attachment #2: Type: text/html, Size: 6924 bytes --]
next prev parent reply other threads:[~2020-05-29 7:19 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-25 12:23 [RFC PATCH] hw/net/e1000e: Do not abort() on invalid PSRCTL register value Philippe Mathieu-Daudé
2020-05-29 6:35 ` Jason Wang
2020-05-29 7:17 ` Philippe Mathieu-Daudé [this message]
2020-05-29 7:27 ` Jason Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAAdtpL5=-e_MwpK0n3X0TeT_XrYOLHfA8gqTrx0WS7yBRqO6MQ@mail.gmail.com' \
--to=f4bug@amsat.org \
--cc=dmitry.fleytman@gmail.com \
--cc=jasowang@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).