Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [Critical] The newly added `config_lock` mutex in `struct mxt_data` is used without being initialized.
- [High] If `mxt_initialize()` aborts due to `shutting_down == true`, it consumes the completion but fails to restore it, leading to a permanent hang.
- [High] The `config_lock` mutex is held across a potentially unbounded wait, defeating the purpose of the `shutting_down` flag.
- [Medium] A time-of-check to time-of-use (TOCTOU) race allows asynchronous firmware loading to start after device removal has begun.

Pre-existing issues:
- [High] Concurrent sysfs writes to the `update_fw` attribute can trigger a double free of the input device.
--

commit a6627582981aef52fbfe146ee37db19f67d402dd
Author: Hendrik Noack [off-list ref]
Input: atmel_mxt_ts: Fix async config use-after-free

This commit attempts to fix a use-after-free by adding a mutex and a
completion to serialize the asynchronous firmware loading and the device
removal process.

[ ... ]

[Severity: High]
Is it safe to hold the config_lock mutex across this wait_for_completion()
call? 

Since the lock is held while waiting for a potentially long firmware load,
and mxt_remove() needs this same lock to set shutting_down to true, would
this cause mxt_remove() to block indefinitely, defeating the early shutdown
check?

+
+		if (data->shutting_down)
+			return -EBUSY;

[Severity: High]
If shutting_down is true, the wait_for_completion() above has already
decremented the completion's counter. 

Returning -EBUSY here without calling complete(&data->config_completion)
leaves the completion consumed. When mxt_remove() later calls
wait_for_completion(), will it hang forever waiting for a completion that
is never restored?

+
+		reinit_completion(&data->config_completion);
+	}
+
 	error = request_firmware_nowait(THIS_MODULE, true, MXT_CFG_NAME,
 					&client->dev, GFP_KERNEL, data,
 					mxt_config_cb);

[Severity: Medium]
Because request_firmware_nowait() is called after the config_lock is
released, is there a race condition here? 

If mxt_remove() runs immediately after the lock is dropped, it will acquire
the lock and set shutting_down to true, but request_firmware_nowait() will
still be invoked, starting a new asynchronous load while the device is
actively being removed.

[ ... ]

[Severity: Critical]
Does the config_lock mutex need to be initialized here? 

The mxt_data structure is allocated with devm_kzalloc(), which 
zero-initializes the memory. Using an uninitialized mutex with 
scoped_guard() will cause a NULL pointer dereference when contention 
occurs. Should there be a call to mutex_init(&data->config_lock)?

[ ... ]

[Severity: High]
This is a pre-existing issue, but looking at mxt_free_input_device(), it
checks if data->input_dev is non-NULL and then unregisters it.

If concurrent sysfs writes to the update_fw attribute occur, multiple
threads can enter mxt_load_fw() and call mxt_free_input_device() at the
same time. Since sysfs doesn't serialize these writes and there is no
internal locking, could both threads pass the NULL check before either
sets it to NULL, resulting in a use-after-free and double-free of the
input device?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523152332.86864-1-hendrik-noack@gmx.de?part=1