Hi,
Liu Xiang <liu.xiang6@xxxxxxxxxx> writes:
> In multi-core SoC, if enable USB endpoint/transmit urb/disable
> USB endpoint repeatedly,it can cause a NULL pointer dereference bug:
>
> Unable to handle kernel NULL pointer dereference at virtual address 00000010
> pgd = d3eb4000
> [00000010] *pgd=00000000
> Internal error: Oops: 5 [#1] PREEMPT SMP
> Pid: 1017, comm: mediaserver
> CPU: 0 Not tainted (3.0.101-ZXICTOS_V2.00.20_P2B4_ZTE_ZX296700_ASIC+ #2)
> PC is at musb_h_disable+0xfc/0x15c
> LR is at musb_h_disable+0xfc/0x15c
> pc : [<c0353de8>] lr : [<c0353de8>] psr: 60070093
> sp : d3e5bc80 ip : 00000027 fp : d3e5bca4
> r10: 00000000 r9 : ffffff94 r8 : 00000080
> r7 : 60070013 r6 : d45900f0 r5 : d4717720 r4 : cee2d860
> r3 : 00000000 r2 : c0875628 r1 : d3e5bbc0 r0 : 0000002a
> Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user
> Control: 18c53c7d Table: b3eb404a DAC: 00000015
> [<c00519d0>] (__dabt_svc+0x70/0xa0) from [<c0353de8>] (musb_h_disable+0xfc/0x15c)
> [<c0353de8>] (musb_h_disable+0xfc/0x15c) from [<c032f160>] (usb_hcd_disable_endpoint+0x3c/0x44)
> [<c032f160>] (usb_hcd_disable_endpoint+0x3c/0x44) from [<c0331504>] (usb_disable_endpoint+0x64/0x7c)
> [<c0331504>] (usb_disable_endpoint+0x64/0x7c) from [<c0331564>] (usb_disable_interface+0x48/0x58)
> [<c0331564>] (usb_disable_interface+0x48/0x58) from [<c0331bcc>] (usb_set_interface+0x158/0x274)
> [<c0331bcc>] (usb_set_interface+0x158/0x274) from [<c03a8a30>] (uvc_video_enable+0x78/0x9c)
> [<c03a8a30>] (uvc_video_enable+0x78/0x9c) from [<c03a66c4>] (uvc_v4l2_do_ioctl+0xd08/0x12ec)
> [<c03a66c4>] (uvc_v4l2_do_ioctl+0xd08/0x12ec) from [<c036c044>] (video_usercopy+0x144/0x500)
> [<c036c044>] (video_usercopy+0x144/0x500) from [<c03a5444>] (uvc_v4l2_ioctl+0x2c/0x74)
> [<c03a5444>] (uvc_v4l2_ioctl+0x2c/0x74) from [<c036b208>] (v4l2_ioctl+0x94/0x154)
> [<c036b208>] (v4l2_ioctl+0x94/0x154) from [<c0128628>] (do_vfs_ioctl+0x84/0x5ac)
> [<c0128628>] (do_vfs_ioctl+0x84/0x5ac) from [<c0128bc8>] (sys_ioctl+0x78/0x80)
> [<c0128bc8>] (sys_ioctl+0x78/0x80) from [<c0052000>] (ret_fast_syscall+0x0/0x30)
>
> Considering the following execution sequence:
> CPU0 CPU1
> musb_urb_dequeue
> spin_lock_irqsave(&musb->lock, flags);
> ready = qh->is_ready;
> qh->is_ready = 0;
> musb_giveback(musb, urb, 0);
> spin_unlock(&musb->lock);
> zx296702_musb_interrupt
> musb_interrupt
> spin_lock_irqsave(&musb->lock, flags);
> musb_advance_schedule
> ready = qh->is_ready;
> qh->is_ready = 0;
> musb_giveback(musb, urb, status);
> spin_unlock(&musb->lock);
> spin_lock(&musb->lock);
> qh->is_ready = ready;
> spin_unlock_irqrestore(&musb->lock, flags);
> spin_lock(&musb->lock);
> qh->is_ready = ready;
>
> When musb_urb_dequeue is called finally, the qh->is_ready has already
> been set to 0 in error.Thus the recycling qh job in musb_urb_dequeue
> can not be done. That results in accessing empty qh in musb_h_disable.
> So the qh in musb_h_disable should be checked.If the qh is emtpy,
> then recycle it and go to exit directly.
>
> Signed-off-by: Liu Xiang <liu.xiang6@xxxxxxxxxx>
> ---
> drivers/usb/musb/musb_host.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> index 795a45b..438f5b4 100644
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -2515,7 +2515,12 @@ musb_h_disable(struct usb_hcd *hcd, struct usb_host_endpoint *hep)
> qh->is_ready = 0;
> if (musb_ep_get_qh(qh->hw_ep, is_in) == qh) {
> urb = next_urb(qh);
> -
> + if (urb == NULL) {
!urb would be nicer here.
Also, adding Bin as he'll become the new MUSB maintainer.
> + qh->hep->hcpriv = NULL;
> + list_del(&qh->ring);
> + kfree(qh);
> + goto exit;
> + }
> /* make software (then hardware) stop ASAP */
> if (!urb->unlinked)
> urb->status = -ESHUTDOWN;
> --
> 1.9.1
>
>
> --
> 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
--
balbi
Attachment:
signature.asc
Description: PGP signature
