Hi David,
Thanks for the review.
> -----Original Message-----
> From: Cohen, David A
> Sent: Thursday, April 04, 2013 10:03 AM
> To: Aguirre Rodriguez, Sergio A
> Cc: linux-usb@xxxxxxxxxxxxxxx; Sarah Sharp; Kanigeri, Hari K
> Subject: Re: [PATCH v2] xhci: prevent from potential null pointer
> dereference on failed init
>
> Hi Sergio,
>
> On 04/04/2013 09:57 AM, Sergio Aguirre wrote:
> > It is possible that we fail on xhci_mem_init, just before doing the
> > INIT_LIST_HEAD, and calling xhci_mem_cleanup.
> >
> > Problem is that, the list_for_each_entry_safe macro, dereferences next
> > assuming is not NULL (which is the case for a uninitialized list).
> >
> > Let's protect from that.
> >
> > Signed-off-by: Sergio Aguirre <sergio.a.aguirre.rodriguez@xxxxxxxxx>
> > ---
> > drivers/usb/host/xhci-mem.c | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> > index 6dc238c..d110aeb 100644
> > --- a/drivers/usb/host/xhci-mem.c
> > +++ b/drivers/usb/host/xhci-mem.c
> > @@ -1820,9 +1820,12 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
> > scratchpad_free(xhci);
> >
> > spin_lock_irqsave(&xhci->lock, flags);
> > - list_for_each_entry_safe(dev_info, next, &xhci->lpm_failed_devs,
> list) {
> > - list_del(&dev_info->list);
> > - kfree(dev_info);
> > + if (xhci->lpm_failed_devs.next) {
>
> IMO this check could be done without holding spin lock.
> At this point, either we initialized list_head or not.
I see your point.
However, I think I'll go with Bjørn's suggestion to init the lists at the
beginning, to avoid doing these null pointer checks altogether.
Will resend v3 in a couple of seconds.
Regards,
Sergio
>
> Br, David
>
> > + list_for_each_entry_safe(dev_info, next,
> > + &xhci->lpm_failed_devs, list) {
> > + list_del(&dev_info->list);
> > + kfree(dev_info);
> > + }
> > }
> > spin_unlock_irqrestore(&xhci->lock, flags);
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
