Hi Greg,
Here are musb fixes for v4.17-rc4 to fix two NULL pointer dereference cases. Please let me know if any change is needed.
Regards, -Bin. ----
Bin Liu (2): usb: musb: host: fix potential NULL pointer dereference usb: musb: trace: fix NULL pointer dereference in musb_g_tx()
drivers/usb/musb/musb_gadget.c | 3 ++- drivers/usb/musb/musb_host.c | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-)
musb_start_urb() doesn't check the pass-in parameter if it is NULL. But in musb_bulk_nak_timeout() the parameter passed to musb_start_urb() is returned from first_qh(), which could be NULL.
So wrap the musb_start_urb() call here with a if condition check to avoid the potential NULL pointer dereference.
Fixes: f283862f3b5cb("usb: musb: NAK timeout scheme on bulk TX endpoint")
Cc: stable@vger.kernel.org # v3.7+ Signed-off-by: Bin Liu b-liu@ti.com --- drivers/usb/musb/musb_host.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index 4fa372c845e1..e7f99d55922a 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -990,7 +990,9 @@ static void musb_bulk_nak_timeout(struct musb *musb, struct musb_hw_ep *ep, /* set tx_reinit and schedule the next qh */ ep->tx_reinit = 1; } - musb_start_urb(musb, is_in, next_qh); + + if (next_qh) + musb_start_urb(musb, is_in, next_qh); } }
On Mon, Apr 30, 2018 at 11:20:53AM -0500, Bin Liu wrote:
musb_start_urb() doesn't check the pass-in parameter if it is NULL. But in musb_bulk_nak_timeout() the parameter passed to musb_start_urb() is returned from first_qh(), which could be NULL.
So wrap the musb_start_urb() call here with a if condition check to avoid the potential NULL pointer dereference.
Fixes: f283862f3b5cb("usb: musb: NAK timeout scheme on bulk TX endpoint")
Nit, you forgot a ' ', this should be: f283862f3b5c ("usb: musb: NAK timeout scheme on bulk TX endpoint")
You also had one extra id value in there, odd. I'll edit this by hand...
greg k-h
On Mon, Apr 30, 2018 at 09:42:15AM -0700, Greg Kroah-Hartman wrote:
On Mon, Apr 30, 2018 at 11:20:53AM -0500, Bin Liu wrote:
musb_start_urb() doesn't check the pass-in parameter if it is NULL. But in musb_bulk_nak_timeout() the parameter passed to musb_start_urb() is returned from first_qh(), which could be NULL.
So wrap the musb_start_urb() call here with a if condition check to avoid the potential NULL pointer dereference.
Fixes: f283862f3b5cb("usb: musb: NAK timeout scheme on bulk TX endpoint")
Nit, you forgot a ' ', this should be: f283862f3b5c ("usb: musb: NAK timeout scheme on bulk TX endpoint")
Sorry, thanks.
You also had one extra id value in there, odd. I'll edit this by
Not sure why 'git blame' gives that one extra on my computer. I will see if I will figure it out...
hand...
Thanks, -Bin.
On Mon, Apr 30, 2018 at 01:11:34PM -0500, Bin Liu wrote:
On Mon, Apr 30, 2018 at 09:42:15AM -0700, Greg Kroah-Hartman wrote:
On Mon, Apr 30, 2018 at 11:20:53AM -0500, Bin Liu wrote:
musb_start_urb() doesn't check the pass-in parameter if it is NULL. But in musb_bulk_nak_timeout() the parameter passed to musb_start_urb() is returned from first_qh(), which could be NULL.
So wrap the musb_start_urb() call here with a if condition check to avoid the potential NULL pointer dereference.
Fixes: f283862f3b5cb("usb: musb: NAK timeout scheme on bulk TX endpoint")
Nit, you forgot a ' ', this should be: f283862f3b5c ("usb: musb: NAK timeout scheme on bulk TX endpoint")
Sorry, thanks.
You also had one extra id value in there, odd. I'll edit this by
Not sure why 'git blame' gives that one extra on my computer. I will see if I will figure it out...
Why use 'git blame'? I use: git show -s --abbrev-commit --abbrev=12 --pretty=format:"%h ("%s")%n"
to generate these types of lines.
thanks,
greg k-h
On Mon, Apr 30, 2018 at 11:24:48AM -0700, Greg Kroah-Hartman wrote:
On Mon, Apr 30, 2018 at 01:11:34PM -0500, Bin Liu wrote:
On Mon, Apr 30, 2018 at 09:42:15AM -0700, Greg Kroah-Hartman wrote:
On Mon, Apr 30, 2018 at 11:20:53AM -0500, Bin Liu wrote:
musb_start_urb() doesn't check the pass-in parameter if it is NULL. But in musb_bulk_nak_timeout() the parameter passed to musb_start_urb() is returned from first_qh(), which could be NULL.
So wrap the musb_start_urb() call here with a if condition check to avoid the potential NULL pointer dereference.
Fixes: f283862f3b5cb("usb: musb: NAK timeout scheme on bulk TX endpoint")
Nit, you forgot a ' ', this should be: f283862f3b5c ("usb: musb: NAK timeout scheme on bulk TX endpoint")
Sorry, thanks.
You also had one extra id value in there, odd. I'll edit this by
Not sure why 'git blame' gives that one extra on my computer. I will see if I will figure it out...
Why use 'git blame'? I use: git show -s --abbrev-commit --abbrev=12 --pretty=format:"%h ("%s")%n"
to generate these types of lines.
My ~/.gitconfig already has abbrev = 12
but I used 'git blame' to find the commit id which introduces the bug, then just directly copied the commit id from there... only until now know 'git blame' gives 13 chars...
I will create a shortcut for the command you gave above to avoid this 13 chars problem from now on.
Thanks, -Bin.
The usb_request pointer could be NULL in musb_g_tx(), where the tracepoint call would trigger the NULL pointer dereference failure when parsing the members of the usb_request pointer.
Move the tracepoint call to where the usb_request pointer is already checked to solve the issue.
Fixes: fc78003e5345a("usb: musb: gadget: add usb-request tracepoints")
Cc: stable@vger.kernel.org # v4.8+ Signed-off-by: Bin Liu b-liu@ti.com --- drivers/usb/musb/musb_gadget.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c index e564695c6c8d..71c5835ea9cd 100644 --- a/drivers/usb/musb/musb_gadget.c +++ b/drivers/usb/musb/musb_gadget.c @@ -417,7 +417,6 @@ void musb_g_tx(struct musb *musb, u8 epnum) req = next_request(musb_ep); request = &req->request;
- trace_musb_req_tx(req); csr = musb_readw(epio, MUSB_TXCSR); musb_dbg(musb, "<== %s, txcsr %04x", musb_ep->end_point.name, csr);
@@ -456,6 +455,8 @@ void musb_g_tx(struct musb *musb, u8 epnum) u8 is_dma = 0; bool short_packet = false;
+ trace_musb_req_tx(req); + if (dma && (csr & MUSB_TXCSR_DMAENAB)) { is_dma = 1; csr |= MUSB_TXCSR_P_WZC_BITS;
linux-stable-mirror@lists.linaro.org
-
Bin Liu -
Greg Kroah-Hartman