From: Felipe Balbi <balbi@kernel.org>
To: Liu Xiang <liu.xiang6@zte.com.cn>,
linux-usb@vger.kernel.org, Bin Liu <b-liu@ti.com>
Cc: linux-kernel@vger.kernel.org, balbi@ti.com,
gregkh@linuxfoundation.org, Liu Xiang <liu.xiang6@zte.com.cn>
Subject: Re: [PATCH] usb: musb: host: Fix NULL pointer dereference in SMP environment
Date: Tue, 23 Feb 2016 06:32:45 +0200 [thread overview]
Message-ID: <874md0kpw2.fsf@ti.com> (raw)
In-Reply-To: <1455899913-3847-1-git-send-email-liu.xiang6@zte.com.cn>
[-- Attachment #1: Type: text/plain, Size: 4504 bytes --]
Hi,
Liu Xiang <liu.xiang6@zte.com.cn> 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@zte.com.cn>
> ---
> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
prev parent reply other threads:[~2016-02-23 12:33 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-19 16:38 [PATCH] usb: musb: host: Fix NULL pointer dereference in SMP environment Liu Xiang
2016-02-23 4:32 ` Felipe Balbi [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=874md0kpw2.fsf@ti.com \
--to=balbi@kernel.org \
--cc=b-liu@ti.com \
--cc=balbi@ti.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=liu.xiang6@zte.com.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.