Re: [PATCH 08/10] Input: synaptics-rmi4 - F54 style and typo fixes

[sashiko-bot@kernel.org]

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

New issues:
- [Low] The newly added comments incorrectly describe the lock coverage for `status_mutex` and `data_mutex`.

Pre-existing issues:
- [High] Embedding a `struct video_device` in a `devm_kzalloc`-managed structure causes a use-after-free on device unbind.
--

commit 7a52e1aa5971437b387ed577aeae2fd8da17b2ba
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: synaptics-rmi4 - F54 style and typo fixes

This commit cleans up style issues in rmi_f54.c reported by checkpatch.pl.
It converts bit shifts to BIT() macros, aligns assignments and arguments,
fixes typos, and adds mutex documentation comments.

> diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
> index 93526feea5636..850e1742c480a 100644
> --- a/drivers/input/rmi4/rmi_f54.c
> +++ b/drivers/input/rmi4/rmi_f54.c
[ ... ]
> @@ -109,8 +109,8 @@ struct f54_data {
>  	int report_error;
>  
>  	bool is_busy;
> -	struct mutex status_mutex;
> -	struct mutex data_mutex;
> +	struct mutex status_mutex; /* Protects is_busy and command state */
> +	struct mutex data_mutex;   /* Protects report_data buffer */

[Severity: Low]
Are these comments accurate regarding the lock coverage?

Looking at rmi_f54_request_report() and rmi_f54_work(), it appears that
is_busy, cmd_done, and timeout are explicitly modified while holding
data_mutex, not status_mutex.

status_mutex appears to be used to serialize calls to rmi_f54_buffer_queue()
instead.

[ ... ]
> @@ -709,7 +709,7 @@ static int rmi_f54_probe(struct rmi_function *fn)
>  	f54->max_report_size = array3_size(tx, rx, sizeof(u16));
>  	f54->report_data = devm_kzalloc(&fn->dev, f54->max_report_size,
>  					GFP_KERNEL);
> -	if (f54->report_data == NULL)
> +	if (!f54->report_data)
>  		return -ENOMEM;
>  
>  	INIT_DELAYED_WORK(&f54->work, rmi_f54_work);

[Severity: High]
This isn't a bug introduced by this patch, but does allocating the f54_data
structure with devm_kzalloc() earlier in rmi_f54_probe() lead to a
use-after-free on device unbind?

If a userspace process opens the v4l2 device node, and concurrently the
physical device is hot-unplugged or unbound via sysfs, devm will
automatically free the f54_data memory.

However, since the release callback for the embedded struct video_device is
set to video_device_release_empty, the v4l2 core retains a reference to the
embedded video_device.

Could it then access the already-freed memory during ioctls or when the last
file descriptor is closed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626051802.4033172-1-dmitry.torokhov@gmail.com?part=8