Skip to contentSkip to conversation

FAILED: patch "[PATCH] usb: typec: ucsi: ccg: Fix use-after-free of ucsi on remove" failed to apply to 5.15-stable tree

PATCH New [email protected] 2 participants 6 messages Jul 14 – Jul 20

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.15.y
git checkout FETCH_HEAD
git cherry-pick -x 1f0bdc2884b67de337215079bba166df0cdf4ac5
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026071422-comment-stardust-654c@gregkh' --subject-prefix 'PATCH 5.15.y' 'HEAD^..'

Possible dependencies:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 1f0bdc2884b67de337215079bba166df0cdf4ac5 Mon Sep 17 00:00:00 2001
From: Fan Wu <[email protected]>
Date: Tue, 16 Jun 2026 13:20:11 +0000
Subject: [PATCH] usb: typec: ucsi: ccg: Fix use-after-free of ucsi on remove

The threaded IRQ handler ccg_irq_handler() calls ucsi_notify_common(), which on a connector-change event calls ucsi_connector_change() and schedules connector work. In ucsi_ccg_remove(), ucsi_destroy() frees uc->ucsi (kfree) before free_irq() is called, so a handler invocation already in flight may access the freed object after ucsi_destroy().

  CPU 0 (remove)            | CPU 1 (threaded IRQ)
    ucsi_destroy(uc->ucsi)  |   ccg_irq_handler()
      kfree(ucsi) // FREE   |     ucsi_notify_common(uc->ucsi) // USE

Move free_irq() before ucsi_destroy() in the remove path. It is kept after ucsi_unregister(): ucsi_unregister() cancels connector work whose handler issues GET_CONNECTOR_STATUS through ucsi_send_command_common(), which waits for a completion that is signalled from the IRQ handler, so the IRQ must stay active until that work has been cancelled.

The probe error path already orders free_irq() before ucsi_destroy().

This bug was found by static analysis.

Fixes e32fd989ac1c ("usb: typec: ucsi: ccg: Move to the new API")
Cc stable <[email protected]>
drivers/usb/typec/ucsi/ucsi_ccg.c +1 -1
index d46ca942026e..91c2958a708c 100644
@@ -1521,8 +1521,8 @@ static void ucsi_ccg_remove(struct i2c_client *client)
 	cancel_work_sync(&uc->work);
 	pm_runtime_disable(uc->dev);
 	ucsi_unregister(uc->ucsi);
-	ucsi_destroy(uc->ucsi);
 	free_irq(uc->irq, uc);
+	ucsi_destroy(uc->ucsi);
 }
 
 static const struct of_device_id ucsi_ccg_of_match_table[] = {
maintainer
#1 lore raw

PATCH 1/5 drm/i2c/sil164: Drop no-op remove function

From: Uwe Kleine-König <[email protected]>

[ Upstream commit eb09882d64807c24228ed1d005dbbae70860ca3f ]

A remove callback that just returns 0 is equivalent to no callback at all as can be seen in i2c_device_remove(). So simplify accordingly.

Stable-dep-of: 1f0bdc2884b6 ("usb: typec: ucsi: ccg: Fix use-after-free of ucsi on remove")

---
 drivers/gpu/drm/i2c/sil164_drv.c | 7 -------
 1 file changed, 7 deletions(-)
drivers/gpu/drm/i2c/sil164_drv.c +0 -7
index 741886b5441973..1bc0b5de44995d 100644
@@ -370,12 +370,6 @@ sil164_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	return 0;
 }
 
-static int
-sil164_remove(struct i2c_client *client)
-{
-	return 0;
-}
-
 static struct i2c_client *
 sil164_detect_slave(struct i2c_client *client)
 {
@@ -427,7 +421,6 @@ MODULE_DEVICE_TABLE(i2c, sil164_ids);
 static struct drm_i2c_encoder_driver sil164_driver = {
 	.i2c_driver = {
 		.probe = sil164_probe,
-		.remove = sil164_remove,
 		.driver = {
 			.name = "sil164",
 		},
Signature · 1 line
2.53.0
maintainer
#2 lore raw

PATCH 2/5 leds: lm3697: Remove duplicated error reporting in .remove()

From: Uwe Kleine-König <[email protected]>

[ Upstream commit af89fa11fae1cee4e07453f780e6e5573ef0f477 ]

Returning an error value from an i2c remove callback results in an error message being emitted by the i2c core, but otherwise it doesn't make a difference. The device goes away anyhow and the devm cleanups are called.

As lm3697_remove() already emits an error message on failure and the additional error message by the i2c core doesn't add any useful information, don't pass the error value up the stack. Instead continue to clean up and return 0.

This patch is a preparation for making i2c remove callbacks return void.

Stable-dep-of: 1f0bdc2884b6 ("usb: typec: ucsi: ccg: Fix use-after-free of ucsi on remove")

---
 drivers/leds/leds-lm3697.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/leds/leds-lm3697.c +1 -3
index a8c9322558cc51..3ecf90fbc06c09 100644
@@ -345,10 +345,8 @@ static int lm3697_remove(struct i2c_client *client)
 
 	ret = regmap_update_bits(led->regmap, LM3697_CTRL_ENABLE,
 				 LM3697_CTRL_A_B_EN, 0);
-	if (ret) {
+	if (ret)
 		dev_err(dev, "Failed to disable the device\n");
-		return ret;
-	}
 
 	if (led->enable_gpio)
 		gpiod_direction_output(led->enable_gpio, 0);
Signature · 1 line
2.53.0
maintainer
#3 lore raw

PATCH 3/5 leds: lm3601x: Improve error reporting for problems during .remove()

In reply to Sasha Levin

From: Uwe Kleine-König <[email protected]>

[ Upstream commit 22a23436891886a66d21af3619f4a4e8809f0e0a ]

Returning an error value in an i2c remove callback results in a generic error message being emitted by the i2c core, but otherwise it doesn't make a difference. The device goes away anyhow and the devm cleanups are called.

So instead of triggering the generic i2c error message, emit a more helpful message if a problem occurs and return 0 to suppress the generic message.

This patch is a preparation for making i2c remove callbacks return void.

Stable-dep-of: 1f0bdc2884b6 ("usb: typec: ucsi: ccg: Fix use-after-free of ucsi on remove")

---
 drivers/leds/flash/leds-lm3601x.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/leds/flash/leds-lm3601x.c +8 -3
index 3d127274820173..37e1d6e6868777 100644
@@ -443,10 +443,15 @@ static int lm3601x_probe(struct i2c_client *client)
 static int lm3601x_remove(struct i2c_client *client)
 {
 	struct lm3601x_led *led = i2c_get_clientdata(client);
+	int ret;
 
-	return regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
-			   LM3601X_ENABLE_MASK,
-			   LM3601X_MODE_STANDBY);
+	ret = regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
+				 LM3601X_ENABLE_MASK, LM3601X_MODE_STANDBY);
+	if (ret)
+		dev_warn(&client->dev,
+			 "Failed to put into standby (%pe)\n", ERR_PTR(ret));
+
+	return 0;
 }
 
 static const struct i2c_device_id lm3601x_id[] = {
Signature · 1 line
2.53.0
maintainer
#4 lore raw

PATCH 4/5 gpio: pca953x: Make platform teardown callback return void

In reply to Sasha Levin

From: Uwe Kleine-König <[email protected]>

[ Upstream commit 6a8f359c3132e4f51bdb263ad74ec632c65e55fd ]

All platforms that provide a teardown callback return 0. New users are supposed to not make use of platform support, so there is no functionality lost.

This patch is a preparation for making i2c remove callbacks return void.

Stable-dep-of: 1f0bdc2884b6 ("usb: typec: ucsi: ccg: Fix use-after-free of ucsi on remove")

---
 arch/arm/mach-davinci/board-da850-evm.c | 12 ++++--------
 drivers/gpio/gpio-pca953x.c             | 11 +++--------
 include/linux/platform_data/pca953x.h   |  2 +-
 3 files changed, 8 insertions(+), 17 deletions(-)
arch/arm/mach-davinci/board-da850-evm.c +4 -8
index 1601a74261a67b..71bd7c53048c5c 100644
@@ -517,8 +517,8 @@ static int da850_evm_ui_expander_setup(struct i2c_client *client, unsigned gpio,
 	return ret;
 }
 
-static int da850_evm_ui_expander_teardown(struct i2c_client *client,
-					unsigned gpio, unsigned ngpio, void *c)
+static void da850_evm_ui_expander_teardown(struct i2c_client *client,
+					   unsigned gpio, unsigned ngpio, void *c)
 {
 	platform_device_unregister(&da850_evm_ui_keys_device);
 
@@ -530,8 +530,6 @@ static int da850_evm_ui_expander_teardown(struct i2c_client *client,
 	gpio_free(gpio + DA850_EVM_UI_EXP_SEL_C);
 	gpio_free(gpio + DA850_EVM_UI_EXP_SEL_B);
 	gpio_free(gpio + DA850_EVM_UI_EXP_SEL_A);
-
-	return 0;
 }
 
 /* assign the baseboard expander's GPIOs after the UI board's */
@@ -698,13 +696,11 @@ static int da850_evm_bb_expander_setup(struct i2c_client *client,
 	return ret;
 }
 
-static int da850_evm_bb_expander_teardown(struct i2c_client *client,
-					unsigned gpio, unsigned ngpio, void *c)
+static void da850_evm_bb_expander_teardown(struct i2c_client *client,
+					   unsigned gpio, unsigned ngpio, void *c)
 {
 	platform_device_unregister(&da850_evm_bb_leds_device);
 	platform_device_unregister(&da850_evm_bb_keys_device);
-
-	return 0;
 }
 
 static struct pca953x_platform_data da850_evm_ui_expander_info = {
drivers/gpio/gpio-pca953x.c +3 -8
index a8f6ef4006fef9..94c888510243c2 100644
@@ -1074,20 +1074,15 @@ static int pca953x_remove(struct i2c_client *client)
 {
 	struct pca953x_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct pca953x_chip *chip = i2c_get_clientdata(client);
-	int ret;
 
 	if (pdata && pdata->teardown) {
-		ret = pdata->teardown(client, chip->gpio_chip.base,
-				      chip->gpio_chip.ngpio, pdata->context);
-		if (ret < 0)
-			dev_err(&client->dev, "teardown failed, %d\n", ret);
-	} else {
-		ret = 0;
+		pdata->teardown(client, chip->gpio_chip.base,
+				chip->gpio_chip.ngpio, pdata->context);
 	}
 
 	regulator_disable(chip->regulator);
 
-	return ret;
+	return 0;
 }
 
 #ifdef CONFIG_PM_SLEEP
include/linux/platform_data/pca953x.h +1 -1
index 4eb53e02399786..96c1a14ab3657a 100644
@@ -22,7 +22,7 @@ struct pca953x_platform_data {
 	int		(*setup)(struct i2c_client *client,
 				unsigned gpio, unsigned ngpio,
 				void *context);
-	int		(*teardown)(struct i2c_client *client,
+	void		(*teardown)(struct i2c_client *client,
 				unsigned gpio, unsigned ngpio,
 				void *context);
 	const char	*const *names;
Signature · 1 line
2.53.0
maintainer
#5 lore raw

PATCH 5/5 usb: typec: ucsi: ccg: Fix use-after-free of ucsi on remove

In reply to Sasha Levin

From: Fan Wu <[email protected]>

[ Upstream commit 1f0bdc2884b67de337215079bba166df0cdf4ac5 ]

The threaded IRQ handler ccg_irq_handler() calls ucsi_notify_common(), which on a connector-change event calls ucsi_connector_change() and schedules connector work. In ucsi_ccg_remove(), ucsi_destroy() frees uc->ucsi (kfree) before free_irq() is called, so a handler invocation already in flight may access the freed object after ucsi_destroy().

  CPU 0 (remove)            | CPU 1 (threaded IRQ)
    ucsi_destroy(uc->ucsi)  |   ccg_irq_handler()
      kfree(ucsi) // FREE   |     ucsi_notify_common(uc->ucsi) // USE

Move free_irq() before ucsi_destroy() in the remove path. It is kept after ucsi_unregister(): ucsi_unregister() cancels connector work whose handler issues GET_CONNECTOR_STATUS through ucsi_send_command_common(), which waits for a completion that is signalled from the IRQ handler, so the IRQ must stay active until that work has been cancelled.

The probe error path already orders free_irq() before ucsi_destroy().

This bug was found by static analysis.

Fixes e32fd989ac1c ("usb: typec: ucsi: ccg: Move to the new API")
Cc stable <[email protected]>
---
 drivers/usb/typec/ucsi/ucsi_ccg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/typec/ucsi/ucsi_ccg.c +1 -1
index 68a693d92dd05a..8d69bc5bff20a9 100644
@@ -1418,8 +1418,8 @@ static int ucsi_ccg_remove(struct i2c_client *client)
 	cancel_work_sync(&uc->work);
 	pm_runtime_disable(uc->dev);
 	ucsi_unregister(uc->ucsi);
-	ucsi_destroy(uc->ucsi);
 	free_irq(uc->irq, uc);
+	ucsi_destroy(uc->ucsi);
 
 	return 0;
 }
Signature · 1 line
2.53.0