[PATCH 0/2] usb: musb: sunxi: Fix unbinding and rebinding

[PATCH 0/2] usb: musb: sunxi: Fix unbinding and rebinding

[Rajan Yadav via B4 Relay]

Unbinding musb-sunxi crashes on current mainline: KASAN reports a
use-after-free in devm_usb_phy_release(), followed by a NULL pointer
dereference. This came back in v6.12 with commit 498dbd9aea20 ("usb:
musb: sunxi: Fix accessing an released usb phy"), which removed the 2017
fix for the same oops. Patch 1 lets devres unregister the generic PHY
after the PHY reference has been dropped.

While testing it, binding the musb-hdrc child again, the case that
498dbd9aea20 was about, turned out to fail with -EEXIST: since 2017 the
extcon notifier is registered with devm on the glue device, so it is
never unregistered when the child goes away. Patch 2 pairs the
registration with sunxi_musb_init() and sunxi_musb_exit() again.

The two patches are independent. Both were tested on a KickPi K5C
(Allwinner A133) with v7.4-rc1 and KASAN, without out-of-tree modules,
using the board's device tree, which is not in mainline yet. The
details, and what was not tested, are in each patch.

From reading the code only, drivers/usb/musb/mediatek.c seems to have
the same problem as patch 1: mtk_musb_remove() unregisters the generic
PHY while devm_usb_get_phy() still holds a reference to it. I have no
hardware to test it.

As in the patches, I used Claude Code (Anthropic) to find the causes,
write the fixes and the commit messages, and help with the tests; I
reviewed everything and ran the tests on the board.

Signed-off-by: Rajan Yadav <rajanyadav0307@gmail.com>
---
Rajan Yadav (2):
      usb: musb: sunxi: Unregister the generic PHY after dropping its reference
      usb: musb: sunxi: Unregister the extcon notifier in sunxi_musb_exit()

 drivers/usb/musb/sunxi.c | 43 +++++++++++++++++++++++++++++++------------
 1 file changed, 31 insertions(+), 12 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261110-musb-sunxi-unbind-2ab4d0d6bc76

Best regards,
--  
Rajan Yadav <rajanyadav0307@gmail.com>

[PATCH 1/2] usb: musb: sunxi: Unregister the generic PHY after dropping its reference

[Rajan Yadav via B4 Relay]

From: Rajan Yadav <rajanyadav0307@gmail.com>

Unbinding musb-sunxi oopses on v7.4-rc1 (KickPi K5C, Allwinner A133,
KASAN enabled, no out-of-tree modules):

  # echo 5100000.usb > /sys/bus/platform/drivers/musb-sunxi/unbind
  BUG: KASAN: slab-use-after-free in devm_usb_phy_release+0x2c/0x68
  Read of size 8 at addr ffff0000057ef080 by task sh/329
  ...
  Allocated by task 48:
   ...
   devm_kmalloc+0x5c/0x140
   usb_phy_generic_probe+0x30/0x184
   ...
   usb_phy_generic_register+0x9c/0xd8
   sunxi_musb_probe+0x310/0x5c8
  ...
  Freed by task 329:
   ...
   platform_device_unregister+0x24/0x40
   usb_phy_generic_unregister+0x10/0x1c
   sunxi_musb_remove+0x44/0x54
   ...
  Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
  ...
  pc : devm_usb_phy_release+0x44/0x68

sunxi_musb_remove() unregisters the usb_phy_generic device. That unbinds
its driver, which frees the struct usb_phy_generic holding the struct
usb_phy. The reference that sunxi_musb_probe() took with
devm_usb_get_phy() is only dropped after that, when devres runs, and
usb_put_phy() then reads x->dev->driver->owner from the freed PHY with
dev->driver already NULL. The probe error paths unregister the generic
PHY with the devres reference still held in the same way.

Commit 6ed05c68cbca ("usb: musb: sunxi: Explicitly release USB PHY on
exit") fixed this oops in 2017 by calling devm_usb_put_phy() in
sunxi_musb_exit(). Commit 498dbd9aea20 ("usb: musb: sunxi: Fix
accessing an released usb phy") removed that call again, because
sunxi_musb_exit() also runs when only the musb-hdrc child is unbound,
and the PHY must stay valid for the next bind. That brought the oops
back.

Keep the reference for the lifetime of the glue device, as 498dbd9aea20
intended, and let devres unregister the generic PHY device as well. The
action is added before devm_usb_get_phy(), so devres drops the
reference first, on unbind and on the probe error paths.

Tested on the K5C with v7.4-rc1 and KASAN: unbinding musb-sunxi gives
no KASAN report and no oops, the generic PHY device, musb-hdrc and the
UDC are removed and come back on bind, and 11 unbind/bind cycles stayed
clean. The probe error paths were not exercised, and no USB gadget
traffic was tested (no data connection to a host).

Claude Code (Anthropic) found the cause from the oops, wrote this fix
and the commit message, and helped with the tests; I reviewed the fix
and ran the tests on the board.

Fixes: 498dbd9aea20 ("usb: musb: sunxi: Fix accessing an released usb phy")
Cc: stable@vger.kernel.org
Signed-off-by: Rajan Yadav <rajanyadav0307@gmail.com>
---
 drivers/usb/musb/sunxi.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index a6bd3e968cc..d880c3758c9 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -674,6 +674,11 @@ static const struct musb_hdrc_config sunxi_musb_hdrc_config_4eps = {
 	.ram_bits	= SUNXI_MUSB_RAM_BITS,
 };
 
+static void sunxi_musb_unregister_usb_phy(void *usb_phy)
+{
+	usb_phy_generic_unregister(usb_phy);
+}
+
 static int sunxi_musb_probe(struct platform_device *pdev)
 {
 	struct musb_hdrc_platform_data	pdata;
@@ -768,11 +773,21 @@ static int sunxi_musb_probe(struct platform_device *pdev)
 		return PTR_ERR(glue->usb_phy);
 	}
 
+	/*
+	 * Registered before devm_usb_get_phy(), so that devres drops the
+	 * reference to the PHY before the PHY device is unregistered.
+	 */
+	ret = devm_add_action_or_reset(&pdev->dev,
+				       sunxi_musb_unregister_usb_phy,
+				       glue->usb_phy);
+	if (ret)
+		return ret;
+
 	glue->xceiv = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
 	if (IS_ERR(glue->xceiv)) {
 		ret = PTR_ERR(glue->xceiv);
 		dev_err(&pdev->dev, "Error getting usb-phy %d\n", ret);
-		goto err_unregister_usb_phy;
+		return ret;
 	}
 
 	platform_set_drvdata(pdev, glue);
@@ -792,23 +807,17 @@ static int sunxi_musb_probe(struct platform_device *pdev)
 	if (IS_ERR(glue->musb_pdev)) {
 		ret = PTR_ERR(glue->musb_pdev);
 		dev_err(&pdev->dev, "Error registering musb dev: %d\n", ret);
-		goto err_unregister_usb_phy;
+		return ret;
 	}
 
 	return 0;
-
-err_unregister_usb_phy:
-	usb_phy_generic_unregister(glue->usb_phy);
-	return ret;
 }
 
 static void sunxi_musb_remove(struct platform_device *pdev)
 {
 	struct sunxi_glue *glue = platform_get_drvdata(pdev);
-	struct platform_device *usb_phy = glue->usb_phy;
 
 	platform_device_unregister(glue->musb_pdev);
-	usb_phy_generic_unregister(usb_phy);
 }
 
 static const struct sunxi_musb_cfg sun4i_a10_musb_cfg = {

-- 
2.34.1

[PATCH 2/2] usb: musb: sunxi: Unregister the extcon notifier in sunxi_musb_exit()

[Rajan Yadav via B4 Relay]

From: Rajan Yadav <rajanyadav0307@gmail.com>

Binding the musb-hdrc child again after it was unbound fails on v7.4-rc1
(KickPi K5C, Allwinner A133):

  # echo musb-hdrc.1.auto > /sys/bus/platform/drivers/musb-hdrc/unbind
  # echo musb-hdrc.1.auto > /sys/bus/platform/drivers/musb-hdrc/bind
  notifier callback sunxi_musb_host_notifier already registered
  WARNING: kernel/notifier.c:24 at notifier_chain_register+0x98/0xf8, CPU#1: sh/312
  ...
   notifier_chain_register+0x98/0xf8 (P)
   raw_notifier_chain_register+0x14/0x20
   extcon_register_notifier+0xa4/0xe0
   devm_extcon_register_notifier+0x64/0xe0
   sunxi_musb_init+0x170/0x280
   musb_probe+0x330/0x125c
  ...
  musb-hdrc musb-hdrc.1.auto: error -EEXIST: musb_init_controller failed
  musb-hdrc musb-hdrc.1.auto: probe with driver musb-hdrc failed with error -17

sunxi_musb_init() runs each time the child is probed, but it registers
the extcon notifier with devm_extcon_register_notifier() on the glue
device. The notifier stays registered after the child is unbound, and
the next sunxi_musb_init() registers the same notifier_block again. It
also stays registered while no musb exists, so an extcon event can
still queue glue->work after sunxi_musb_exit() cancelled it.

Commit bb1d1ce8c74e ("usb: musb: sunxi: Uses the resource-managed
extcon API when registering extcon notifier") introduced this; before
it, the notifier was unregistered in sunxi_musb_exit() and on the
sunxi_musb_init() error path. Restore that, and unregister it before
cancel_work_sync() so that the notifier cannot queue the work again.

Tested on the K5C with v7.4-rc1 and KASAN. The bind failed as above
both without and with the previous patch. With both patches, three
unbind/bind cycles of musb-hdrc bring back the UDC each time, without
warnings or KASAN reports, and unbinding the glue device afterwards is
clean as well. This patch was not tested without the previous one, and
the sunxi_musb_init() error path was not exercised.

Claude Code (Anthropic) found the cause, wrote this fix and the commit
message, and helped with the tests; I reviewed the fix and ran the
tests on the board.

Fixes: bb1d1ce8c74e ("usb: musb: sunxi: Uses the resource-managed extcon API when registering extcon notifier")
Cc: stable@vger.kernel.org
Signed-off-by: Rajan Yadav <rajanyadav0307@gmail.com>
---
 drivers/usb/musb/sunxi.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index d880c3758c9..e9107ae9d34 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -246,15 +246,19 @@ static int sunxi_musb_init(struct musb *musb)
 
 	writeb(SUNXI_MUSB_VEND0_PIO_MODE, musb->mregs + SUNXI_MUSB_VEND0);
 
-	/* Register notifier before calling phy_init() */
-	ret = devm_extcon_register_notifier(glue->dev, glue->extcon,
-					EXTCON_USB_HOST, &glue->host_nb);
+	/*
+	 * Register notifier before calling phy_init(). It is unregistered in
+	 * sunxi_musb_exit(), as this runs again each time the musb-hdrc child
+	 * is bound.
+	 */
+	ret = extcon_register_notifier(glue->extcon, EXTCON_USB_HOST,
+				       &glue->host_nb);
 	if (ret)
 		goto error_reset_assert;
 
 	ret = phy_init(glue->phy);
 	if (ret)
-		goto error_reset_assert;
+		goto error_unregister_notifier;
 
 	musb->isr = sunxi_musb_interrupt;
 
@@ -263,6 +267,9 @@ static int sunxi_musb_init(struct musb *musb)
 
 	return 0;
 
+error_unregister_notifier:
+	extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
+				   &glue->host_nb);
 error_reset_assert:
 	if (test_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags))
 		reset_control_assert(glue->rst);
@@ -280,6 +287,9 @@ static int sunxi_musb_exit(struct musb *musb)
 
 	pm_runtime_put(musb->controller);
 
+	/* Before cancel_work_sync(), so the notifier cannot requeue the work */
+	extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
+				   &glue->host_nb);
 	cancel_work_sync(&glue->work);
 	if (test_bit(SUNXI_MUSB_FL_PHY_ON, &glue->flags))
 		phy_power_off(glue->phy);

-- 
2.34.1

Re: [PATCH 1/2] usb: musb: sunxi: Unregister the generic PHY after dropping its reference

[sashiko-bot]

> From: Rajan Yadav <rajanyadav0307@gmail.com>
> 
> Unbinding musb-sunxi oopses on v7.4-rc1 (KickPi K5C, Allwinner A133,
> KASAN enabled, no out-of-tree modules):
> 
>   # echo 5100000.usb > /sys/bus/platform/drivers/musb-sunxi/unbind
>   BUG: KASAN: slab-use-after-free in devm_usb_phy_release+0x2c/0x68
>   Read of size 8 at addr ffff0000057ef080 by task sh/329
>   ...
>   Allocated by task 48:
>    ...
>    devm_kmalloc+0x5c/0x140
>    usb_phy_generic_probe+0x30/0x184
>    ...
>    usb_phy_generic_register+0x9c/0xd8
> [ ... ]
> and ran the tests on the board.
> 
> Fixes: 498dbd9aea20 ("usb: musb: sunxi: Fix accessing an released usb phy")
> Cc: stable@vger.kernel.org
> Signed-off-by: Rajan Yadav <rajanyadav0307@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-musb-sunxi-unbind-v1-0-d821584fc4a4@gmail.com?part=1

Re: [PATCH 2/2] usb: musb: sunxi: Unregister the extcon notifier in sunxi_musb_exit()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] usb: musb: sunxi: missing cancel_work_sync() in
  sunxi_musb_init()
--

commit b343ab879ad454784f848e3901439d97d89167a9
Author: Rajan Yadav <rajanyadav0307@gmail.com>

usb: musb: sunxi: Unregister the extcon notifier in sunxi_musb_exit()

This commit switches sunxi_musb_init() from devm_extcon_register_notifier()
to extcon_register_notifier() and unregisters the notifier in
sunxi_musb_exit() and on the sunxi_musb_init() error path.

> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> index d880c3758c91b..e9107ae9d3417 100644
> --- a/drivers/usb/musb/sunxi.c
> +++ b/drivers/usb/musb/sunxi.c
> @@ -246,15 +246,19 @@ static int sunxi_musb_init(struct musb *musb)
>  
>  	writeb(SUNXI_MUSB_VEND0_PIO_MODE, musb->mregs + SUNXI_MUSB_VEND0);
>  
> -	/* Register notifier before calling phy_init() */
> -	ret = devm_extcon_register_notifier(glue->dev, glue->extcon,
> -					EXTCON_USB_HOST, &glue->host_nb);
> +	/*
> +	 * Register notifier before calling phy_init(). It is unregistered in
> +	 * sunxi_musb_exit(), as this runs again each time the musb-hdrc child
> +	 * is bound.
> +	 */
> +	ret = extcon_register_notifier(glue->extcon, EXTCON_USB_HOST,
> +				       &glue->host_nb);
>  	if (ret)
>  		goto error_reset_assert;
>  
>  	ret = phy_init(glue->phy);
>  	if (ret)
> -		goto error_reset_assert;
> +		goto error_unregister_notifier;
>  
>  	musb->isr = sunxi_musb_interrupt;
>  
> @@ -263,6 +267,9 @@ static int sunxi_musb_init(struct musb *musb)
>  
>  	return 0;
>  
> +error_unregister_notifier:
> +	extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
> +				   &glue->host_nb);
>  error_reset_assert:
>  	if (test_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags))
>  		reset_control_assert(glue->rst);

[Severity: Medium]
Should the error_unregister_notifier path also call
cancel_work_sync(&glue->work) after extcon_unregister_notifier(), just like
sunxi_musb_exit() does?

An EXTCON_USB_HOST event arriving between extcon_register_notifier() and a
failing phy_init() schedules glue->work via sunxi_musb_host_notifier().
Because musb-hdrc fails to bind, unbinding the glue device in
sunxi_musb_remove() will not run sunxi_musb_exit() and devres can free
glue while glue->work is still queued or running.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-musb-sunxi-unbind-v1-0-d821584fc4a4@gmail.com?part=2