Re: [PATCH v2 2/2] Input: synaptics-rmi4 - reject a PDT that grows between scans
From: sashiko-bot@kernel.org
Date: 2026-08-24 12:38:02
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Memory leak and kobject lifecycle violation on the error path in `rmi_create_function()`. --
--- Patch [2]: [PATCH v2 2/2] Input: synaptics-rmi4 - reject a PDT that grows between scans ---commit 846b91538dc6072de55b69393a47494582f0c196 Author: Wei Jie Law [off-list ref] Input: synaptics-rmi4 - reject a PDT that grows between scans This commit adds a validation check in rmi_create_function() to reject devices whose interrupt counts grow between PDT scans, preventing an out-of-bounds bitmap write. When the check fails, it frees the function using kfree().
The rejection happens before rmi_register_function(), i.e. before device_initialize() has touched fn->dev, so the partially built function is disposed of with kfree(): put_device() on a kobject that was never initialised warns and then leaves the object behind, because the saturated refcount stops the release from ever running.
[Severity: High]
Is this assumption accurate? Looking at rmi_alloc_function() which is called
right at the beginning of rmi_create_function(), it appears
device_initialize() is actually called during allocation:
drivers/input/rmi4/rmi_bus.c:rmi_alloc_function() {
...
device_initialize(&fn->dev);
dev_set_name(&fn->dev, "%s.fn%02x", dev_name(&rmi_dev->dev), id);
...
}
quoted hunk ↗ jump to hunk
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c index 5d49a9021c7d0..8dcf088a7a819 100644 --- a/drivers/input/rmi4/rmi_driver.c +++ b/drivers/input/rmi4/rmi_driver.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -899,6 +899,24 @@ static int rmi_create_function(struct rmi_device *rmi_dev, fn->irq_pos = *current_irq_count; *current_irq_count += fn->num_of_irqs; + /* + * irq_mask[] was sized from the interrupt count collected by the + * earlier rmi_count_irqs() scan of the PDT. Nothing guarantees that + * this scan sees the same table -- the PDT is read back from the + * device every time -- so a device that grows its interrupt counts + * between the two scans would push these set_bit() calls past the end + * of the flexible array. Refuse the function instead. + */ + if (fn->num_of_irqs > RMI_FN_MAX_IRQS || + fn->irq_pos + fn->num_of_irqs > data->irq_count) { + dev_err(dev, + "F%02X: interrupt count changed between PDT scans (pos %u + %u > %d)\n", + pdt->function_number, fn->irq_pos, fn->num_of_irqs, + data->irq_count); + kfree(fn);
[Severity: High] Does this kfree() bypass the kobject lifecycle rules and cause a memory leak? Since rmi_alloc_function() already initialized the kobject and dynamically allocated the device name string via dev_set_name(), shouldn't we use put_device(&fn->dev) here instead of kfree()? Calling kfree() directly skips kobject_cleanup(), which permanently leaks the heap-allocated name string and violates the driver core requirement that initialized kobjects must be freed via their release method.
+ return -EINVAL; + } + for (i = 0; i < fn->num_of_irqs; i++) set_bit(fn->irq_pos + i, fn->irq_mask);
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260824122733.76321-1-98lawweijie@gmail.com?part=2