Thread (4 messages) 4 messages, 2 authors, 1d ago

[PATCH v2 1/2] Input: atmel_mxt_ts: Fix async config use-after-free

From: Hendrik Noack <hidden>
Date: 2026-05-23 15:23:58
Also in: lkml
Subsystem: atmel maxtouch driver, input (keyboard, mouse, joystick, touchscreen) drivers, the rest · Maintainers: Nick Dyer, Dmitry Torokhov, Linus Torvalds

The Atmel maXTouch driver starts asynchronous config loading from both
probe and a sysfs trigger via request_firmware_nowait() and passes the
devm-managed struct mxt_data as the callback context. If the driver is
removed while an asynchronous request is still pending, devres will free
mxt_data before the firmware callback runs. When the callback eventually
executes, it dereferences the freed mxt_data pointer, leading to a
use-after-free.

This is resolved by introducing a lock and a completion that serialise the
request_firmware_nowait() and remove():

Before calling request_firmware_nowait(), a mutex guards the waiting and
reinitialisation of the completion. If remove() has started, the
asynchronous loading will no longer be started. The completion is
completed at the end of the callback or if request_firmware_nowait()
fails.

In mxt_remove(), the same mutex guards the start of remove(). Afterwards,
the completion of pending request_firmware_nowait() is awaited.

Signed-off-by: Hendrik Noack <redacted>
---
Changes in v2:
- this fix is added to the patch series (thanks sashiko.dev)
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 29 +++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index a9e86ad7ed5e..07690c3bff06 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -347,6 +347,11 @@ struct mxt_data {
 	/* for config update handling */
 	struct completion crc_completion;
 
+	/* for loading config */
+	struct completion config_completion;
+	struct mutex config_lock;
+	bool shutting_down;
+
 	u32 *t19_keymap;
 	unsigned int t19_num_keys;
 
@@ -2221,8 +2226,11 @@ static int mxt_configure_objects(struct mxt_data *data,
 
 static void mxt_config_cb(const struct firmware *cfg, void *ctx)
 {
-	mxt_configure_objects(ctx, cfg);
+	struct mxt_data *data = ctx;
+
+	mxt_configure_objects(data, cfg);
 	release_firmware(cfg);
+	complete(&data->config_completion);
 }
 
 static int mxt_initialize(struct mxt_data *data)
@@ -2271,12 +2279,22 @@ static int mxt_initialize(struct mxt_data *data)
 	if (error)
 		return error;
 
+	scoped_guard(mutex, &data->config_lock) {
+		wait_for_completion(&data->config_completion);
+
+		if (data->shutting_down)
+			return -EBUSY;
+
+		reinit_completion(&data->config_completion);
+	}
+
 	error = request_firmware_nowait(THIS_MODULE, true, MXT_CFG_NAME,
 					&client->dev, GFP_KERNEL, data,
 					mxt_config_cb);
 	if (error) {
 		dev_err(&client->dev, "Failed to invoke firmware loader: %d\n",
 			error);
+		complete(&data->config_completion);
 		return error;
 	}
 
@@ -3237,6 +3255,9 @@ static int mxt_probe(struct i2c_client *client)
 	init_completion(&data->bl_completion);
 	init_completion(&data->reset_completion);
 	init_completion(&data->crc_completion);
+	init_completion(&data->config_completion);
+	complete(&data->config_completion);
+	data->shutting_down = false;
 
 	data->suspend_mode = dmi_check_system(chromebook_T9_suspend_dmi) ?
 		MXT_SUSPEND_T9_CTRL : MXT_SUSPEND_DEEP_SLEEP;
@@ -3342,6 +3363,12 @@ static void mxt_remove(struct i2c_client *client)
 {
 	struct mxt_data *data = i2c_get_clientdata(client);
 
+	scoped_guard(mutex, &data->config_lock) {
+		data->shutting_down = true;
+	}
+
+	wait_for_completion(&data->config_completion);
+
 	disable_irq(data->irq);
 	mxt_free_input_device(data);
 	mxt_free_object_table(data);
-- 
2.43.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help