Re: [PATCH V5 04/14] i3c: master: Fix use-after-free of master->this

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Pre-existing devres lifetime mismatch causing Use-After-Free in i3c_masterdev_release(), exacerbated by the patch's newly added kfree().
- [High] Pre-existing IDR state corruption due to missing error handling around i3cbus->id in probe failure path.
--

--- Patch [4]: [PATCH V5 04/14] i3c: master: Fix use-after-free of master->this ---
commit aad15513dbf2d559bf893789d55a331f522383d1
Author: Adrian Hunter <adrian.hunter@intel.com>
Subject: i3c: master: Fix use-after-free of master->this

The patch fixes a use-after-free by keeping master->this alive until
i3c_masterdev_release(). It also resets master->this and bus.cur_master to
NULL on the error path in i3c_master_set_info() before freeing the
allocated device.

Cc: stable@vger.kernel.org

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index abb582645a2ed..2357874bb9d6a 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -842,6 +842,11 @@ static struct attribute *i3c_masterdev_attrs[] = {
>  };
>  ATTRIBUTE_GROUPS(i3c_masterdev);
>  
> +static void i3c_master_free_i3c_dev(struct i3c_dev_desc *dev)
> +{
> +	kfree(dev);
> +}
> +
>  static void i3c_masterdev_release(struct device *dev)
>  {
>  	struct i3c_master_controller *master = dev_to_i3cmaster(dev);
> @@ -854,6 +859,8 @@ static void i3c_masterdev_release(struct device *dev)
>  	i3c_bus_cleanup(bus);

[Severity: High]
This isn't a bug introduced by this patch, but does the error path in probe
corrupt the IDR if idr_alloc() fails?

If idr_alloc() fails in i3c_bus_init(), i3cbus->id is not updated and remains
at its kzalloc-initialized value of 0.

The error path in i3c_master_register() calls put_device(&master->dev), which
invokes this i3c_masterdev_release() callback.

That unconditionally calls i3c_bus_cleanup(bus), which executes
idr_remove(&i3c_bus_idr, i3cbus->id). This effectively deletes ID 0 from the
IDR, even though that ID might belong to another successfully initialized bus.

>  
>  	fwnode_handle_put(dev->fwnode);
> +
> +	i3c_master_free_i3c_dev(master->this);

[Severity: High]
This is a pre-existing issue, but can this devres lifetime mismatch lead
to a use-after-free when sysfs files are held open?

The i3c_master_controller struct is typically allocated via devm_kzalloc()
by the parent driver. If an unprivileged user holds a sysfs file open (like
the mode or current_master attributes), device_unregister() drops a
reference, but the sysfs node prevents the kobject from being released.

When the driver is unbound and remove() returns, devres frees the master
struct.

Later, when the sysfs file is closed, the refcount drops to 0 and
i3c_masterdev_release() executes, dereferencing the already-freed master
struct to access master->wq, bus, and the newly added
i3c_master_free_i3c_dev(master->this).

>  }

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807145638.168865-1-adrian.hunter@intel.com?part=4

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c