On Tue, Aug 06, 2019 at 06:23:38AM -0700, Guenter Roeck wrote:
> On 8/2/19 7:11 AM, Gerd Hoffmann wrote:
> > On Wed, Jul 31, 2019 at 01:08:50PM +0200, Philippe Mathieu-Daudé wrote:
> > > On 7/30/19 7:45 PM, Guenter Roeck wrote:
> > > > The following assert is seen once in a while while resetting the
> > > > Linux kernel.
> > > >
> > > > qemu-system-x86_64: hw/usb/core.c:734: usb_ep_get:
> > > > Assertion `dev != NULL' failed.
> > > >
> > > > The call to usb_ep_get() originates from ehci_execute().
> > > > Analysis and debugging shows that p->queue->dev can indeed be NULL
> > > > in this function. Add check for this condition and return an error
> > > > if it is seen.
> > >
> > > Your patch is not wrong as it corrects your case, but I wonder why we
> > > get there. This assert seems to have catched a bug.
> >
> > https://bugzilla.redhat.com//show_bug.cgi?id=1715801 maybe.
> >
> > > Gerd, shouldn't we call usb_packet_cleanup() in ehci_reset() rather than
> > > ehci_finalize()? Then we shouldn't need this patch.
> >
> > The two ehci_queues_rip_all() calls in ehci_reset() should clean up
> > everything
> > properly.
> >
> > Can you try the patch below to see whenever a ehci_find_device failure is
> > the
> > root cause?
> >
> > thanks,
> > Gerd
> >
> > diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
> > index 62dab0592fa2..2b0a57772ed5 100644
> > --- a/hw/usb/hcd-ehci.c
> > +++ b/hw/usb/hcd-ehci.c
> > @@ -1644,6 +1644,10 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState
> > *ehci, int async)
> > q->dev = ehci_find_device(q->ehci,
> > get_field(q->qh.epchar,
> > QH_EPCHAR_DEVADDR));
> > }
> > + if (q->dev == NULL) {
> > + fprintf(stderr, "%s: device %d not found\n", __func__,
> > + get_field(q->qh.epchar, QH_EPCHAR_DEVADDR));
> > + }
> Turns out I end up seeing that message hundreds of times early on each boot,
> no matter which architecture. It is quite obviously a normal operating
> condition.
Yep, as long as the queue is not active this is completely harmless.
So we need to check a bit later. In execute() looks a bit too late
though, we don't have a good backup plan then.
Does the patch below solve the problem without bad side effects?
thanks,
Gerd
>From 5980eaad23f675a2d509d0c55e288793619761bc Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kra...@redhat.com>
Date: Tue, 13 Aug 2019 13:37:09 +0200
Subject: [PATCH] ehci: try fix queue->dev null ptr dereference
Reported-by: Guenter Roeck <li...@roeck-us.net>
Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
---
hw/usb/hcd-ehci.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 62dab0592fa2..5f089f30054b 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1834,6 +1834,9 @@ static int ehci_state_fetchqtd(EHCIQueue *q)
ehci_set_state(q->ehci, q->async, EST_EXECUTING);
break;
}
+ } else if (q->dev == NULL) {
+ ehci_trace_guest_bug(q->ehci, "no device attached to queue");
+ ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH);
} else {
p = ehci_alloc_packet(q);
p->qtdaddr = q->qtdaddr;
--
2.18.1
