USB: backports to 4.19

PATCH In review Johan Hovold 2 participants 9 messages Sep 6, 2022 – Sep 11, 2022 Stable branch

lore

Share & tools

lore

Johan Hovold

author

2022-09-06 14:20 UTC #0 lore raw

cover USB: backports to 4.19

And here are the corresponding backports to 4.19.

Johan

Johan Hovold (4):
usb: dwc3: fix PHY disable sequence
usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup
USB: serial: ch341: fix lost character on LCR updates
USB: serial: ch341: fix disabled rx timer on older devices

 drivers/usb/dwc3/core.c      | 19 ++++++++++---------
 drivers/usb/dwc3/dwc3-qcom.c | 14 +++++++++++++-
 drivers/usb/dwc3/host.c      |  1 +
 drivers/usb/serial/ch341.c   | 15 +++++++++++++--
 4 files changed, 37 insertions(+), 12 deletions(-)

Signature · 1 line

2.35.1

Johan Hovold

author

2022-09-06 14:30 UTC #1 lore raw

PATCH 1/4 usb: dwc3: fix PHY disable sequence

From: Johan Hovold <[email protected]>

commit d2ac7bef95c9ead307801ccb6cb6dfbeb14247bf upstream.

Generic PHYs must be powered-off before they can be tore down.

Similarly, suspending legacy PHYs after having powered them off makes no sense.

Fix the dwc3_core_exit() (e.g. called during suspend) and open-coded dwc3_probe() error-path sequences that got this wrong.

Note that this makes dwc3_core_exit() match the dwc3_core_init() error path with respect to powering off the PHYs.

Fixes 03c1fd622f72 ("usb: dwc3: core: add phy cleanup for probe error handling")

Fixes c499ff71ff2a ("usb: dwc3: core: re-factor init and exit paths")

Cc [email protected] # 4.8

Reviewed-by Andrew Halaney <[email protected]>

Reviewed-by Matthias Kaehlcke <[email protected]>

Reviewed-by Manivannan Sadhasivam <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

Link https://lore.kernel.org/r/[email protected] open here

Signed-off-by Greg Kroah-Hartman <[email protected]>

[ johan: adjust context to 4.19 ]

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/dwc3/core.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

drivers/usb/dwc3/core.c +10 -9

index 984faecdd7ec..465aabb6e96c 100644
@@ -673,15 +673,16 @@ static void dwc3_core_exit(struct dwc3 *dwc)
 {
 	dwc3_event_buffers_cleanup(dwc);
 
+	usb_phy_set_suspend(dwc->usb2_phy, 1);
+	usb_phy_set_suspend(dwc->usb3_phy, 1);
+	phy_power_off(dwc->usb2_generic_phy);
+	phy_power_off(dwc->usb3_generic_phy);
+
 	usb_phy_shutdown(dwc->usb2_phy);
 	usb_phy_shutdown(dwc->usb3_phy);
 	phy_exit(dwc->usb2_generic_phy);
 	phy_exit(dwc->usb3_generic_phy);
 
-	usb_phy_set_suspend(dwc->usb2_phy, 1);
-	usb_phy_set_suspend(dwc->usb3_phy, 1);
-	phy_power_off(dwc->usb2_generic_phy);
-	phy_power_off(dwc->usb3_generic_phy);
 	clk_bulk_disable(dwc->num_clks, dwc->clks);
 	clk_bulk_unprepare(dwc->num_clks, dwc->clks);
 	reset_control_assert(dwc->reset);

@@ -1509,16 +1510,16 @@ static int dwc3_probe(struct platform_device *pdev)
 	dwc3_debugfs_exit(dwc);
 	dwc3_event_buffers_cleanup(dwc);
 
-	usb_phy_shutdown(dwc->usb2_phy);
-	usb_phy_shutdown(dwc->usb3_phy);
-	phy_exit(dwc->usb2_generic_phy);
-	phy_exit(dwc->usb3_generic_phy);
-
 	usb_phy_set_suspend(dwc->usb2_phy, 1);
 	usb_phy_set_suspend(dwc->usb3_phy, 1);
 	phy_power_off(dwc->usb2_generic_phy);
 	phy_power_off(dwc->usb3_generic_phy);
 
+	usb_phy_shutdown(dwc->usb2_phy);
+	usb_phy_shutdown(dwc->usb3_phy);
+	phy_exit(dwc->usb2_generic_phy);
+	phy_exit(dwc->usb3_generic_phy);
+
 	dwc3_ulpi_exit(dwc);
 
 err4:

Signature · 1 line

2.35.1

Johan Hovold

author

2022-09-06 14:16 UTC #2 lore raw

PATCH 2/4 usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup

In reply to Johan Hovold

From: Johan Hovold <[email protected]>

commit a872ab303d5ddd4c965f9cd868677781a33ce35a upstream.

The Qualcomm dwc3 runtime-PM implementation checks the xhci platform-device pointer in the wakeup-interrupt handler to determine whether the controller is in host mode and if so triggers a resume.

After a role switch in OTG mode the xhci platform-device would have been freed and the next wakeup from runtime suspend would access the freed memory.

Note that role switching is executed from a freezable workqueue, which guarantees that the pointer is stable during suspend.

Also note that runtime PM has been broken since commit 2664deb09306 ("usb: dwc3: qcom: Honor wakeup enabled/disabled state"), which incidentally also prevents this issue from being triggered.

Fixes a4333c3a6ba9 ("usb: dwc3: Add Qualcomm DWC3 glue driver")

Cc [email protected] # 4.18

Reviewed-by Matthias Kaehlcke <[email protected]>

Reviewed-by Manivannan Sadhasivam <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

Link https://lore.kernel.org/r/[email protected] open here

Signed-off-by Greg Kroah-Hartman <[email protected]>

[ johan: adjust context for 5.4 ]

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/dwc3/dwc3-qcom.c | 14 +++++++++++++-
 drivers/usb/dwc3/host.c      |  1 +
 2 files changed, 14 insertions(+), 1 deletion(-)

drivers/usb/dwc3/dwc3-qcom.c +13 -1

index 5bb5384f3612..9d5320562e81 100644
@@ -173,6 +173,14 @@ static int dwc3_qcom_register_extcon(struct dwc3_qcom *qcom)
 	return 0;
 }
 
+/* Only usable in contexts where the role can not change. */
+static bool dwc3_qcom_is_host(struct dwc3_qcom *qcom)
+{
+	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+
+	return dwc->xhci;
+}
+
 static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
 {
 	if (qcom->hs_phy_irq) {

@@ -280,7 +288,11 @@ static irqreturn_t qcom_dwc3_resume_irq(int irq, void *data)
 	if (qcom->pm_suspended)
 		return IRQ_HANDLED;
 
-	if (dwc->xhci)
+	/*
+	 * This is safe as role switching is done from a freezable workqueue
+	 * and the wakeup interrupts are disabled as part of resume.
+	 */
+	if (dwc3_qcom_is_host(qcom))
 		pm_runtime_resume(&dwc->xhci->dev);
 
 	return IRQ_HANDLED;

drivers/usb/dwc3/host.c +1 -0

index 1a3878a3be78..124e9f80dccd 100644
@@ -142,4 +142,5 @@ void dwc3_host_exit(struct dwc3 *dwc)
 	phy_remove_lookup(dwc->usb3_generic_phy, "usb3-phy",
 			  dev_name(dwc->dev));
 	platform_device_unregister(dwc->xhci);
+	dwc->xhci = NULL;
 }

Signature · 1 line

2.35.1

Johan Hovold

author

2022-09-06 14:20 UTC #3 lore raw

PATCH 3/4 USB: serial: ch341: fix lost character on LCR updates

In reply to Johan Hovold

commit 8e83622ae7ca481c76c8fd9579877f6abae64ca2 upstream.

Disable LCR updates for pre-0x30 devices which use a different (unknown) protocol for line control and where the current register write causes the next received character to be lost.

Note that updating LCR using the INIT command has no effect on these devices either.

Reported-by Jonathan Woithe <[email protected]>

Tested-by Jonathan Woithe <[email protected]>

Link https://lore.kernel.org/r/[email protected] open here

Fixes 4e46c410e050 ("USB: serial: ch341: reinitialize chip on reconfiguration")

Fixes 55fa15b5987d ("USB: serial: ch341: fix baud rate and line-control handling")

Cc [email protected] # 4.10

Signed-off-by Johan Hovold <[email protected]>

[ johan: adjust context to 4.19 ]

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/ch341.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

drivers/usb/serial/ch341.c +8 -1

index f789b60ed8c1..58b5fd95b29f 100644
@@ -96,6 +96,8 @@ struct ch341_private {
 	u8 mcr;
 	u8 msr;
 	u8 lcr;
+
+	u8 version;
 };
 
 static void ch341_set_termios(struct tty_struct *tty,

@@ -181,6 +183,9 @@ static int ch341_set_baudrate_lcr(struct usb_device *dev,
 	if (r)
 		return r;
 
+	if (priv->version < 0x30)
+		return 0;
+
 	r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x2518, lcr);
 	if (r)
 		return r;

@@ -232,7 +237,9 @@ static int ch341_configure(struct usb_device *dev, struct ch341_private *priv)
 	r = ch341_control_in(dev, CH341_REQ_READ_VERSION, 0, 0, buffer, size);
 	if (r < 0)
 		goto out;
-	dev_dbg(&dev->dev, "Chip version: 0x%02x\n", buffer[0]);
+
+	priv->version = buffer[0];
+	dev_dbg(&dev->dev, "Chip version: 0x%02x\n", priv->version);
 
 	r = ch341_control_out(dev, CH341_REQ_SERIAL_INIT, 0, 0);
 	if (r < 0)

Signature · 1 line

2.35.1

Johan Hovold

author

2022-09-06 14:16 UTC #4 lore raw

PATCH 4/4 USB: serial: ch341: fix disabled rx timer on older devices

In reply to Johan Hovold

commit 41ca302a697b64a3dab4676e01d0d11bb184737d upstream.

At least one older CH341 appears to have the RX timer enable bit inverted so that setting it disables the RX timer and prevents the FIFO from emptying until it is full.

Only set the RX timer enable bit for devices with version newer than 0x27 (even though this probably affects all pre-0x30 devices).

Reported-by Jonathan Woithe <[email protected]>

Tested-by Jonathan Woithe <[email protected]>

Link https://lore.kernel.org/r/[email protected] open here

Fixes 4e46c410e050 ("USB: serial: ch341: reinitialize chip on reconfiguration")

Cc [email protected] # 4.10

Signed-off-by Johan Hovold <[email protected]>

[ johan: backport to 5.4 ]

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/ch341.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

drivers/usb/serial/ch341.c +5 -1

index 58b5fd95b29f..7eabb1bfafde 100644
@@ -176,8 +176,12 @@ static int ch341_set_baudrate_lcr(struct usb_device *dev,
 	/*
 	 * CH341A buffers data until a full endpoint-size packet (32 bytes)
 	 * has been received unless bit 7 is set.
+	 *
+	 * At least one device with version 0x27 appears to have this bit
+	 * inverted.
 	 */
-	a |= BIT(7);
+	if (priv->version > 0x27)
+		a |= BIT(7);
 
 	r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x1312, a);
 	if (r)

Signature · 1 line

2.35.1

Johan Hovold

author

2022-09-06 14:44 UTC #5 lore raw

In reply to Johan Hovold

Quoted 7 lines · Johan Hovold

And here are the corresponding backports to 4.19.
Johan
Johan Hovold (4):
usb: dwc3: fix PHY disable sequence

This one needs another spin for 4.14...

Quoted 4 lines

usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup
USB: serial: ch341: fix lost character on LCR updates
USB: serial: ch341: fix disabled rx timer on older devices

But these two should hopefully apply to the older trees too now.

Johan

Greg Kroah-Hartman

maintainer

2022-09-11 05:43 UTC #6 lore raw

In reply to Johan Hovold

Quoted 11 lines · Johan Hovold

And here are the corresponding backports to 4.19.
Johan
Johan Hovold (4):
usb: dwc3: fix PHY disable sequence
usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup
USB: serial: ch341: fix lost character on LCR updates
USB: serial: ch341: fix disabled rx timer on older devices

Quoted 5 lines

 drivers/usb/dwc3/core.c      | 19 ++++++++++---------
 drivers/usb/dwc3/dwc3-qcom.c | 14 +++++++++++++-
 drivers/usb/dwc3/host.c      |  1 +
 drivers/usb/serial/ch341.c   | 15 +++++++++++++--
 4 files changed, 37 insertions(+), 12 deletions(-)

All backports now queued up, many thanks for doing these.

greg k-h

Johan Hovold

author

2022-09-11 10:29 UTC #7 lore raw

Quoted 12 lines · Greg Kroah-Hartman

Quoted 11 lines · Johan Hovold
And here are the corresponding backports to 4.19.
Johan
Johan Hovold (4):
usb: dwc3: fix PHY disable sequence
usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup
USB: serial: ch341: fix lost character on LCR updates
USB: serial: ch341: fix disabled rx timer on older devices

Quoted 7 lines

Quoted 5 lines

 drivers/usb/dwc3/core.c      | 19 ++++++++++---------
 drivers/usb/dwc3/dwc3-qcom.c | 14 +++++++++++++-
 drivers/usb/dwc3/host.c      |  1 +
 drivers/usb/serial/ch341.c   | 15 +++++++++++++--
 4 files changed, 37 insertions(+), 12 deletions(-)

All backports now queued up, many thanks for doing these.

I believe you can apply the two ch341 backports also to 4.14.

Johan

Greg Kroah-Hartman

maintainer

2022-09-11 11:32 UTC #8 lore raw

Quoted 19 lines · Johan Hovold

Quoted 18 lines · Greg Kroah-Hartman
Quoted 16 lines · Johan Hovold
And here are the corresponding backports to 4.19.
Johan
Johan Hovold (4):
usb: dwc3: fix PHY disable sequence
usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup
USB: serial: ch341: fix lost character on LCR updates
USB: serial: ch341: fix disabled rx timer on older devices
 drivers/usb/dwc3/core.c      | 19 ++++++++++---------
 drivers/usb/dwc3/dwc3-qcom.c | 14 +++++++++++++-
 drivers/usb/dwc3/host.c      |  1 +
 drivers/usb/serial/ch341.c   | 15 +++++++++++++--
 4 files changed, 37 insertions(+), 12 deletions(-)

Quoted 3 lines

Quoted 1 line
All backports now queued up, many thanks for doing these.
I believe you can apply the two ch341 backports also to 4.14.

Thanks, now added.

greg k-h