[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] usb: deal with potential Null pointer retur
From: |
Gerd Hoffmann |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] usb: deal with potential Null pointer returned by usb_ep_get() |
Date: |
Tue, 5 Feb 2019 09:36:55 +0100 |
User-agent: |
NeoMutt/20180716 |
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