Re: [PATCH 3/5] Input: imagis - replace the protocol_b flag with a protocol enum

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

Pre-existing issues:
- [High] Missing i2c_set_clientdata() in probe leads to a NULL pointer dereference during system suspend and resume.
--

--- Patch [3]: [PATCH 3/5] Input: imagis - replace the protocol_b flag with a protocol enum ---
commit 83084be7bf2e640e645cd6ac0416107ab34ef58c
Author: Markuss Broks <[email protected]>
Subject: Input: imagis - replace the protocol_b flag with a protocol enum

This commit replaces the protocol_b flag in the imagis driver with a protocol
enum to describe each reporting format. The change clarifies the naming
convention and prevents a confusing collection of booleans as new chip formats
are added. No functional changes are intended.

> diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
> index 7cc91f97c06e0..6552d97efe32e 100644
> --- a/drivers/input/touchscreen/imagis.c
> +++ b/drivers/input/touchscreen/imagis.c
[ ... ]
> @@ -442,6 +451,7 @@ static const struct imagis_properties imagis_3038h_data = {
>  	.touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
>  	.whoami_cmd = IST3038C_REG_CHIPID,
>  	.whoami_val = IST3038H_WHOAMI,
> +	.protocol = IMAGIS_PROTOCOL_SHARED_REGISTER,
>  };
>  
>  static const struct of_device_id imagis_of_match[] = {

[Severity: High]
This isn't a bug introduced by this patch, but could a missing
i2c_set_clientdata() in probe lead to a NULL pointer dereference during system
suspend and resume?

The imagis_probe() function dynamically allocates the imagis_ts structure but
never stores it in the device's driver data via i2c_set_clientdata().

When the system later suspends or resumes, imagis_suspend() and imagis_resume()
retrieve the client data using i2c_get_clientdata(), which would return NULL.
The code then immediately dereferences this NULL pointer:

drivers/input/touchscreen/imagis.c:imagis_suspend() {
	struct i2c_client *client = to_i2c_client(dev);
	struct imagis_ts *ts = i2c_get_clientdata(client);
	int error;

	guard(mutex)(&ts->input_dev->mutex);
...

Will this cause a kernel panic during suspend and resume cycles?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3