Re: [PATCH V3 02/14] i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode()

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

New issues:
- [Critical] The patch transforms a predictable NULL pointer dereference in=
to an exploitable Use-After-Free of `master->this` when an I3C device outli=
ves its master controller's unregistration.

Pre-existing issues:
- [High] Uninitialized stack memory (`devinfo`) is used for driver matching=
 when `i3cdev->desc` is NULL.
- [High] A Time-of-Check to Time-of-Use (TOCTOU) race condition exists when=
 accessing `i3cdev->desc->boardinfo`, leading to a Use-After-Free or NULL p=
ointer dereference.
--

--- Patch [2]: [PATCH V3 02/14] i3c: Fix unlocked dereference of dev->desc =
in i3c_device_get_supported_xfer_mode() ---
commit 97cfccca08da732124561cbc1aa49b571a84dd8c
Author: Adrian Hunter <[email protected]>

i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer=
_mode()

This patch modifies i3c_device_get_supported_xfer_mode() to use dev->bus
instead of dev->desc to obtain the master controller. This avoids a potenti=
al
unlocked dereference of dev->desc.

> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> index 101eaa77de686..a3778282e84c2 100644
> --- a/drivers/i3c/device.c
> +++ b/drivers/i3c/device.c
> @@ -309,7 +309,7 @@ EXPORT_SYMBOL_GPL(i3c_device_match_id);
>   */
>  u32 i3c_device_get_supported_xfer_mode(struct i3c_device *dev)
>  {
> -	return i3c_dev_get_master(dev->desc)->this->info.hdr_cap | BIT(I3C_SDR);
> +	return i3c_bus_to_i3c_master(dev->bus)->this->info.hdr_cap | BIT(I3C_SD=
R);

[Severity: Critical]
Does this change introduce a use-after-free if the device outlives the mast=
er
controller?

When a master is unregistered, i3c_master_unregister() calls
i3c_master_unregister_i3c_devs() to unregister child devices. If a child
device has an elevated refcount (e.g., from an open file descriptor), its
struct i3c_device outlives the unregistration.

i3c_master_bus_cleanup() then calls i3c_master_detach_free_devs(), which
explicitly frees master->this.

If the child device later calls i3c_device_get_supported_xfer_mode(), it
would dereference the explicitly freed master->this pointer. The previous
code safely faulted on a NULL dev->desc, but this new path appears to access
freed memory.

[ ... ]

[Severity: High]
This is a pre-existing issue, but does i3c_device_match() have a TOCTOU race
condition leading to a use-after-free?

drivers/i3c/master.c:i3c_device_match() {
        if (i3cdev->desc && i3cdev->desc->boardinfo)
                static_addr_method =3D i3cdev->desc->boardinfo->static_addr=
_method;
}

If i3c_device_match() accesses i3cdev->desc without holding the device lock
during concurrent master unregistration, a context switch could occur right
after i3cdev->desc is checked.

If the unregistration thread then frees the descriptor via kfree() in
i3c_master_bus_cleanup(), the subsequent evaluation of
i3cdev->desc->boardinfo will dereference a freed pointer.

[Severity: High]
This isn't a bug introduced by this patch, but can i3c_device_match_id()
evaluate uninitialized stack memory?

During concurrent driver registration and master unregistration, if
i3c_master_unregister_i3c_devs() sets i3cdev->dev->desc =3D NULL,
i3c_device_match() might still proceed to call i3c_device_match_id().

drivers/i3c/device.c:i3c_device_match_id() {
        struct i3c_device_info devinfo;
        ...
        i3c_device_get_info(i3cdev, &devinfo);
       =20
        manuf =3D I3C_PID_MANUF_ID(devinfo.pid);
}

If i3cdev->desc is NULL, i3c_device_get_info() bails out safely without
populating devinfo, leaving the stack memory completely uninitialized when
it is subsequently read for driver matching.

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260804133810.1849=
[email protected]?part=3D2