Re: [Qemu-devel] [PATCH 2/2] usb: deal with potential Null pointer returned by usb_ep_get()

[Gerd Hoffmann]

On Mon, Feb 04, 2019 at 11:50:33AM +0000, Liam Merwick wrote:
> On 31/01/2019 08:03, Gerd Hoffmann wrote:
> > On Wed, Jan 30, 2019 at 02:37:02PM +0000, Liam Merwick wrote:
> > > From: Liam Merwick <address@hidden>
> > > 
> > > usb_ep_get() can return a Null pointer in the (albeit unlikely) case
> > > that a NULL USBDevice is passed in via the 'dev' parameter.
> > That should never ever happen.
> > 
> > > Reported by the Parfait static code analysis tool
> > Try add "assert(dev != NULL)" to usb_ep_get() instead of sprinkling
> > pointless checks all over the place.
> > 
> Adding "assert(dev != NULL)" to usb_ep_get() isn't sufficient for that tool
> unless the 'if (dev== NULL)' check is removed which seems a backwards step
> even if that NULL USBDevice case is impossible.

Looked at the code again.

The usb device emulation (hw/usb/dev-*.c) never ever calls usb_ep_get()
with dev == NULL.  There are some places in usb host adapter emulation
(hw/usb/hcd-*) which might do this.  uhci for example has this ...

        [ ... ]
        USBDevice *dev = uhci_find_device(s, (td->token >> 8) & 0x7f);
        USBEndpoint *ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf);

        if (ep == NULL) {
        [ ... ]

... and uhci_find_device can return NULL.

So, I'd suggest to check all usb_ep_get() callers, fix them if needed,
then remove the 'if (dev== NULL)' check in usb_ep_get() and add the
assert() instead.

cheers,
  Gerd