Before commit df6d7277e552 ("i2c: core: Do not dereference fwnode in struct device"), i2c_unregister_device() only called fwnode_handle_put() on of_node-s in the form of calling of_node_put(client->dev.of_node).
But after this commit the i2c_client's fwnode now unconditionally gets fwnode_handle_put() on it.
When the i2c_client has no primary (ACPI / OF) fwnode but it does have a software fwnode, the software-node will be the primary node and fwnode_handle_put() will put() it.
But for the software fwnode device_remove_software_node() will also put() it leading to a double free:
[ 82.665598] ------------[ cut here ]------------ [ 82.665609] refcount_t: underflow; use-after-free. [ 82.665808] WARNING: CPU: 3 PID: 1502 at lib/refcount.c:28 refcount_warn_saturate+0xba/0x11 ... [ 82.666830] RIP: 0010:refcount_warn_saturate+0xba/0x110 ... [ 82.666962] <TASK> [ 82.666971] i2c_unregister_device+0x60/0x90
Fix this by not calling fwnode_handle_put() when the primary fwnode is a software-node.
Fixes: df6d7277e552 ("i2c: core: Do not dereference fwnode in struct device") Cc: Andy Shevchenko andriy.shevchenko@linux.intel.com Cc: stable@vger.kernel.org Signed-off-by: Hans de Goede hansg@kernel.org --- drivers/i2c/i2c-core-base.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 2ad2b1838f0f..0849aa44952d 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -1066,7 +1066,13 @@ void i2c_unregister_device(struct i2c_client *client) of_node_clear_flag(to_of_node(fwnode), OF_POPULATED); else if (is_acpi_device_node(fwnode)) acpi_device_clear_enumerated(to_acpi_device_node(fwnode)); - fwnode_handle_put(fwnode); + + /* + * If the primary fwnode is a software node it is free-ed by + * device_remove_software_node() below, avoid double-free. + */ + if (!is_software_node(fwnode)) + fwnode_handle_put(fwnode);
device_remove_software_node(&client->dev); device_unregister(&client->dev);
On Sat, Jul 19, 2025 at 08:01:04PM +0200, Hans de Goede wrote:
Before commit df6d7277e552 ("i2c: core: Do not dereference fwnode in struct device"), i2c_unregister_device() only called fwnode_handle_put() on of_node-s in the form of calling of_node_put(client->dev.of_node).
But after this commit the i2c_client's fwnode now unconditionally gets fwnode_handle_put() on it.
When the i2c_client has no primary (ACPI / OF) fwnode but it does have a software fwnode, the software-node will be the primary node and fwnode_handle_put() will put() it.
But for the software fwnode device_remove_software_node() will also put() it leading to a double free:
[ 82.665598] ------------[ cut here ]------------ [ 82.665609] refcount_t: underflow; use-after-free. [ 82.665808] WARNING: CPU: 3 PID: 1502 at lib/refcount.c:28 refcount_warn_saturate+0xba/0x11 ... [ 82.666830] RIP: 0010:refcount_warn_saturate+0xba/0x110 ... [ 82.666962] <TASK> [ 82.666971] i2c_unregister_device+0x60/0x90
Fix this by not calling fwnode_handle_put() when the primary fwnode is a software-node.
Thanks for the fix! It doesn't look elegant, but I think it's all due to the (current) design of the fwnode linked list. Reviewed-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
On Sat, Jul 19, 2025 at 08:01:04PM +0200, Hans de Goede wrote:
Before commit df6d7277e552 ("i2c: core: Do not dereference fwnode in struct device"), i2c_unregister_device() only called fwnode_handle_put() on of_node-s in the form of calling of_node_put(client->dev.of_node).
But after this commit the i2c_client's fwnode now unconditionally gets fwnode_handle_put() on it.
When the i2c_client has no primary (ACPI / OF) fwnode but it does have a software fwnode, the software-node will be the primary node and fwnode_handle_put() will put() it.
But for the software fwnode device_remove_software_node() will also put() it leading to a double free:
Applied to for-next, thanks!
linux-stable-mirror@lists.linaro.org