Re: [PATCH v3 2/2] Input: atmel_mxt_ts: Allow per-machine config via DT compatible

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Concurrent execution of 'mxt_load_fw' via the 'update_fw' sysfs attribute lacks locking, leading to double-free of the input device, TOCTOU races, and interleaved I2C commands.
--

commit eb7b8b6b98800084b750dc1b902ad664e878e3d8
Author: Hendrik Noack <hendrik-noack@gmx.de>

Input: atmel_mxt_ts: Allow per-machine config via DT compatible

The Atmel maXTouch config is machine-dependent, different panels and
layouts require different configs even when the same controller and
firmware is used.

> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index a88dc7e6827c7..5ac123d4ed036 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
[ ... ]
> @@ -2291,12 +2351,9 @@ static int mxt_initialize(struct mxt_data *data)
>  		return -EBUSY;
>  	}
>  
> -	error = request_firmware_nowait(THIS_MODULE, true, MXT_CFG_NAME,
> -					&client->dev, GFP_KERNEL, data,
> -					mxt_config_cb);
> +	error = mxt_invoke_config_loader(data, true);
>  	if (error) {
> -		dev_err(&client->dev, "Failed to invoke firmware loader: %d\n",
> -			error);
> +		dev_err(&client->dev, "Failed to invoke config loader: %d\n", error);
>  		complete(&data->config_completion);
>  		return error;
>  	}

[Severity: High]
This is a pre-existing issue, but I noticed that mxt_update_fw_store()
invokes mxt_load_fw() without any locking to serialize concurrent execution.

Since sysfs write operations don't hold a global lock like device_lock,
could multiple threads concurrently writing to the update_fw attribute race
when checking and setting data->in_bootloader?

If so, could both threads concurrently execute mxt_free_input_device(),
observe a non-null data->input_dev, and invoke input_unregister_device() on
the same pointer before it is set to NULL?

This seems like it might lead to a double-free or use-after-free since
input_unregister_device() drops the reference count.

Could this also race with PM callbacks like mxt_suspend() attempting to
acquire input_dev->mutex, operating on freed memory if
mxt_free_input_device() is executed concurrently?

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