On Tue, 23 Jun 2009, Maulik Mankad wrote:
> This patch fixes possible NULL pointer dereference issues in MUSB gadget code.
>
> Signed-off-by: Maulik Mankad <x0082077@xxxxxx>
> CC: Felipe Balbi <felipe.balbi@xxxxxxxxx>
> CC: David Brownell <david-b@xxxxxxxxxxx>
>
>
> Index: linux-2.6/drivers/usb/musb/musb_gadget.c
> ===================================================================
> --- linux-2.6.orig/drivers/usb/musb/musb_gadget.c
> +++ linux-2.6/drivers/usb/musb/musb_gadget.c
> @@ -110,6 +110,9 @@ __acquires(ep->musb->lock)
>
> req = to_musb_request(request);
>
> + if (!req)
> + return;
> +
> list_del(&request->list);
> if (req->request.status == -EINPROGRESS)
> req->request.status = status;
Is it really possible for this routine to be called with a NULL request
pointer? There doesn't seem to be any point in doing so; if it is
possible then the caller should be changed, not this routine.
> @@ -1014,6 +1020,12 @@ static int musb_gadget_disable(struct us
> int status = 0;
>
> musb_ep = to_musb_ep(ep);
> +
> + if (!musb_ep) {
> + status = -EINVAL;
> + return status;
> + }
> +
> musb = musb_ep->musb;
> epnum = musb_ep->current_epnum;
> epio = musb->endpoints[epnum].regs;
And is it really possible for this routine to be called with a NULL ep
pointer?
Without otherwise commenting on the correctness of this patch, I'd like
to point out that these two changes are not robust. They depend on the
details of how the structures are laid out.
The first hunk should test whether request is NULL, not whether req is.
That's because to_musb_request(NULL) would yield a non-NULL pointer if
the usb_request member occurred anywhere after the start of the
musb_request structure. Similarly, the second hunk should check ep,
not musb_ep.
Alan Stern
--
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
