| Message ID | 1564508710-22909-1-git-send-email-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ehci: Ensure that device is not NULL before calling usb_ep_get | expand |
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. Gerd, shouldn't we call usb_packet_cleanup() in ehci_reset() rather than ehci_finalize()? Then we shouldn't need this patch. > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > hw/usb/hcd-ehci.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c > index 62dab05..c759f3e 100644 > --- a/hw/usb/hcd-ehci.c > +++ b/hw/usb/hcd-ehci.c > @@ -1348,6 +1348,11 @@ static int ehci_execute(EHCIPacket *p, const char *action) > return -1; > } > > + if (p->queue->dev == NULL) { > + ehci_trace_guest_bug(p->queue->ehci, "No device attached to queue\n"); > + return -1; > + } > + > if (get_field(p->qtd.token, QTD_TOKEN_TBYTES) > BUFF_SIZE) { > ehci_trace_guest_bug(p->queue->ehci, > "guest requested more bytes than allowed"); >
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. > > Gerd, shouldn't we call usb_packet_cleanup() in ehci_reset() rather than > ehci_finalize()? Then we shouldn't need this patch. > If you send me an alternate patch, I'll be happy to insert it into my build to give it some test coverage. Thanks, Guenter > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > --- > > hw/usb/hcd-ehci.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c > > index 62dab05..c759f3e 100644 > > --- a/hw/usb/hcd-ehci.c > > +++ b/hw/usb/hcd-ehci.c > > @@ -1348,6 +1348,11 @@ static int ehci_execute(EHCIPacket *p, const char *action) > > return -1; > > } > > > > + if (p->queue->dev == NULL) { > > + ehci_trace_guest_bug(p->queue->ehci, "No device attached to queue\n"); > > + return -1; > > + } > > + > > if (get_field(p->qtd.token, QTD_TOKEN_TBYTES) > BUFF_SIZE) { > > ehci_trace_guest_bug(p->queue->ehci, > > "guest requested more bytes than allowed"); > >
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)); + } if (async && (q->qh.epchar & QH_EPCHAR_H)) {
On Fri, Aug 02, 2019 at 04:11:49PM +0200, 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)); > + } I had tried that, but this does happen as standard behavior for some architectures (I didn't write down where exactly since I thought it must be normal). But, sure, I'll add a log message. Guenter > > if (async && (q->qh.epchar & QH_EPCHAR_H)) { >
On Fri, Aug 02, 2019 at 09:46:26AM -0700, Guenter Roeck wrote: > On Fri, Aug 02, 2019 at 04:11:49PM +0200, 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)); > > + } > > I had tried that, but this does happen as standard behavior for some > architectures (I didn't write down where exactly since I thought it > must be normal). But, sure, I'll add a log message. > With the log message added, I see it a lot when booting riscv64 images from usb-ehci. ehci_state_fetchqh: device 0 not found It looks like this happens for each usb access (a whopping 800+ times for a simple boot test). So it is definitely a very common condition. The relevant qemu command line is something like -usb -device usb-ehci,id=ehci -device usb-storage,bus=ehci.0,drive=d0 \ -drive file=rootfs.ext2,if=none,id=d0,format=raw The image works fine otherwise, so I thought that the condition is normal. Guenter
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. Guenter
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 <kraxel@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 <linux@roeck-us.net> Signed-off-by: Gerd Hoffmann <kraxel@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;
On Tue, Aug 13, 2019 at 01:42:03PM +0200, Gerd Hoffmann wrote: > 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? > I reverted my patch and applied the patch below to my builds of qemu, for both v4.0 and v4.1, and installed it in my test bed. I'll let you know how it goes. Thanks, Guenter > thanks, > Gerd > > From 5980eaad23f675a2d509d0c55e288793619761bc Mon Sep 17 00:00:00 2001 > From: Gerd Hoffmann <kraxel@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 <linux@roeck-us.net> > Signed-off-by: Gerd Hoffmann <kraxel@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 >
On 8/13/19 4:42 AM, Gerd Hoffmann wrote: > 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 <kraxel@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 <linux@roeck-us.net> > Signed-off-by: Gerd Hoffmann <kraxel@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; > That seems to be working as intended. I have not seen a crash since I applied it. I tested it on top of v4.0 and v4.1. Tested-by: Guenter Roeck <linux@roeck-us.net> Guenter
Hi, > > 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? > That seems to be working as intended. I have not seen a crash > since I applied it. I tested it on top of v4.0 and v4.1. Thanks. Send as formal patch now. cheers, Gerd
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 62dab05..c759f3e 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1348,6 +1348,11 @@ static int ehci_execute(EHCIPacket *p, const char *action) return -1; } + if (p->queue->dev == NULL) { + ehci_trace_guest_bug(p->queue->ehci, "No device attached to queue\n"); + return -1; + } + if (get_field(p->qtd.token, QTD_TOKEN_TBYTES) > BUFF_SIZE) { ehci_trace_guest_bug(p->queue->ehci, "guest requested more bytes than allowed");
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. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- hw/usb/hcd-ehci.c | 5 +++++ 1 file changed, 5 insertions(+)