[PATCH 0/2] usb: renesas_usbhs: fix issues on specific situations

[PATCH 0/2] usb: renesas_usbhs: fix issues on specific situations

[Yoshihiro Shimoda]

This patch set is based on the latest Felipe's usb.git / testing/fixes branch.
(commit id = 50c763f8c1bac0dc00f7788a75f227276c0efd54)

Yoshihiro Shimoda (2):
  usb: renesas_usbhs: fix NULL pointer dereference in xfer_work()
  usb: renesas_usbhs: protect the CFIFOSEL setting in usbhsg_ep_enable()

 drivers/usb/renesas_usbhs/fifo.c       | 18 ++++++++++++++----
 drivers/usb/renesas_usbhs/mod_gadget.c |  9 ++++++++-
 2 files changed, 22 insertions(+), 5 deletions(-)

-- 
1.9.1

[PATCH 1/2] usb: renesas_usbhs: fix NULL pointer dereference in xfer_work()

[Yoshihiro Shimoda]

This patch fixes an issue that the xfer_work() is possible to cause
NULL pointer dereference if the usb cable is disconnected while data
transfer is running.

In such case, a gadget driver may call usb_ep_disable()) before
xfer_work() is actually called. In this case, the usbhs_pkt_pop()
will call usbhsf_fifo_unselect(), and then usbhs_pipe_to_fifo()
in xfer_work() will return NULL.

Fixes: e73a989 ("usb: renesas_usbhs: add DMAEngine support")
Cc: <stable@vger.kernel.org> # v3.1+
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/usb/renesas_usbhs/fifo.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
index 7be4e7d..280ed5f 100644
--- a/drivers/usb/renesas_usbhs/fifo.c
+++ b/drivers/usb/renesas_usbhs/fifo.c
@@ -810,20 +810,27 @@ static void xfer_work(struct work_struct *work)
 {
 	struct usbhs_pkt *pkt = container_of(work, struct usbhs_pkt, work);
 	struct usbhs_pipe *pipe = pkt->pipe;
-	struct usbhs_fifo *fifo = usbhs_pipe_to_fifo(pipe);
+	struct usbhs_fifo *fifo;
 	struct usbhs_priv *priv = usbhs_pipe_to_priv(pipe);
 	struct dma_async_tx_descriptor *desc;
-	struct dma_chan *chan = usbhsf_dma_chan_get(fifo, pkt);
+	struct dma_chan *chan;
 	struct device *dev = usbhs_priv_to_dev(priv);
 	enum dma_transfer_direction dir;
+	unsigned long flags;
 
+	usbhs_lock(priv, flags);
+	fifo = usbhs_pipe_to_fifo(pipe);
+	if (!fifo)
+		goto xfer_work_end;
+
+	chan = usbhsf_dma_chan_get(fifo, pkt);
 	dir = usbhs_pipe_is_dir_in(pipe) ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV;
 
 	desc = dmaengine_prep_slave_single(chan, pkt->dma + pkt->actual,
 					pkt->trans, dir,
 					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
 	if (!desc)
-		return;
+		goto xfer_work_end;
 
 	desc->callback		= usbhsf_dma_complete;
 	desc->callback_param	= pipe;
@@ -831,7 +838,7 @@ static void xfer_work(struct work_struct *work)
 	pkt->cookie = dmaengine_submit(desc);
 	if (pkt->cookie < 0) {
 		dev_err(dev, "Failed to submit dma descriptor\n");
-		return;
+		goto xfer_work_end;
 	}
 
 	dev_dbg(dev, "  %s %d (%d/ %d)\n",
@@ -842,6 +849,9 @@ static void xfer_work(struct work_struct *work)
 	usbhs_pipe_set_trans_count_if_bulk(pipe, pkt->trans);
 	dma_async_issue_pending(chan);
 	usbhs_pipe_enable(pipe);
+
+xfer_work_end:
+	usbhs_unlock(priv, flags);
 }
 
 /*
-- 
1.9.1

[PATCH 2/2] usb: renesas_usbhs: protect the CFIFOSEL setting in usbhsg_ep_enable()

[Yoshihiro Shimoda]

This patch fixes an issue that the CFIFOSEL register value is possible
to be changed by usbhsg_ep_enable() wrongly. And then, a data transfer
using CFIFO may not work correctly.

For example:
 # modprobe g_multi file=usb-storage.bin
 # ifconfig usb0 192.168.1.1 up
 (During the USB host is sending file to the mass storage)
 # ifconfig usb0 down

In this case, since the u_ether.c may call usb_ep_enable() in
eth_stop(), if the renesas_usbhs driver is also using CFIFO for
mass storage, the mass storage may not work correctly.

So, this patch adds usbhs_lock() and usbhs_unlock() calling in
usbhsg_ep_enable() to protect CFIFOSEL register. This is because:
 - CFIFOSEL.CURPIPE = 0 is also needed for the pipe configuration
 - The CFIFOSEL (fifo->sel) is already protected by usbhs_lock()

Fixes: 97664a207bc2 ("usb: renesas_usbhs: shrink spin lock area")
Cc: <stable@vger.kernel.org> # v3.1+
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/usb/renesas_usbhs/mod_gadget.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
index 30345c2..50f3363 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -585,6 +585,9 @@ static int usbhsg_ep_enable(struct usb_ep *ep,
 	struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
 	struct usbhs_pipe *pipe;
 	int ret = -EIO;
+	unsigned long flags;
+
+	usbhs_lock(priv, flags);
 
 	/*
 	 * if it already have pipe,
@@ -593,7 +596,8 @@ static int usbhsg_ep_enable(struct usb_ep *ep,
 	if (uep->pipe) {
 		usbhs_pipe_clear(uep->pipe);
 		usbhs_pipe_sequence_data0(uep->pipe);
-		return 0;
+		ret = 0;
+		goto usbhsg_ep_enable_end;
 	}
 
 	pipe = usbhs_pipe_malloc(priv,
@@ -621,6 +625,9 @@ static int usbhsg_ep_enable(struct usb_ep *ep,
 		ret = 0;
 	}
 
+usbhsg_ep_enable_end:
+	usbhs_unlock(priv, flags);
+
 	return ret;
 }
 
-- 
1.9.1

RE: [PATCH 0/2] usb: renesas_usbhs: fix issues on specific situations

[Yoshihiro Shimoda]

Hi Felipe,

Would you review this patch set?

Best regards,
Yoshihiro Shimoda

> From: Yoshihiro Shimoda
> Sent: Wednesday, June 08, 2016 4:33 PM
> To: balbi@kernel.org
> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; linux-renesas-soc@vger.kernel.org; stable@vger.kernel.org;
> Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Subject: [PATCH 0/2] usb: renesas_usbhs: fix issues on specific situations
> 
> This patch set is based on the latest Felipe's usb.git / testing/fixes branch.
> (commit id = 50c763f8c1bac0dc00f7788a75f227276c0efd54)
> 
> Yoshihiro Shimoda (2):
>   usb: renesas_usbhs: fix NULL pointer dereference in xfer_work()
>   usb: renesas_usbhs: protect the CFIFOSEL setting in usbhsg_ep_enable()
> 
>  drivers/usb/renesas_usbhs/fifo.c       | 18 ++++++++++++++----
>  drivers/usb/renesas_usbhs/mod_gadget.c |  9 ++++++++-
>  2 files changed, 22 insertions(+), 5 deletions(-)
> 
> --
> 1.9.1

RE: [PATCH 0/2] usb: renesas_usbhs: fix issues on specific situations

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 152 bytes --]

Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> writes:

> Hi Felipe,
>
> Would you review this patch set?

both in my queue.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]