Thread (22 messages) 22 messages, 3 authors, 2026-06-29

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

From: sashiko-bot@kernel.org
Date: 2026-06-26 05:29:03

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 [off-list ref]

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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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.

[ ... ]
quoted hunk ↗ jump to hunk
@@ -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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help