Re: [PATCH 18/23] drm/xe/i2c: use platform_device_set_fwnode()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing `cancel_work_sync(&i2c->work)` in teardown path leads to Use-After-Free.
- [High] Missing reference count on I2C adapter across asynchronous handoff leads to Use-After-Free.
- [High] Unhandled I2C adapter removal leaves dangling client pointers which are double-freed during module unload.
--

commit a13cd1dfe26fa249e61eac41fbff512943d0184b
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

drm/xe/i2c: use platform_device_set_fwnode()

Ahead of reworking the reference counting logic for platform devices,
encapsulate the assignment of the firmware node for dynamically allocated
platform devices with the provided helper.

> diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
> index 706783863d07d..af4ebd93ad8e6 100644
> --- a/drivers/gpu/drm/xe/xe_i2c.c
> +++ b/drivers/gpu/drm/xe/xe_i2c.c
> @@ -123,7 +123,7 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
>  	}
>  
>  	pdev->dev.parent = i2c->drm_dev;
> -	pdev->dev.fwnode = fwnode;
> +	platform_device_set_fwnode(pdev, fwnode);
>  	i2c->adapter_node = fwnode;
>  	i2c->pdev = pdev;

[Severity: High]
This isn't a bug introduced by this patch, but does the teardown path miss a
cancel_work_sync(&i2c->work)?

The xe_i2c_probe() function initializes i2c->work, which is scheduled
asynchronously in xe_i2c_notifier() when the I2C adapter is added. However,
the driver's teardown function xe_i2c_remove() fails to call
cancel_work_sync(&i2c->work) before returning.

Because i2c is allocated via devm_kzalloc(), the memory is automatically
freed by the devres framework shortly after xe_i2c_remove() completes. If the
background work is still pending or actively executing during or after this
teardown, xe_i2c_client_work() will dereference the freed i2c object to
access i2c->adapter and i2c->ep.addr[1], which could cause a use-after-free.

[Severity: High]
This is a pre-existing issue, but does the I2C adapter need its reference
count incremented across the asynchronous handoff?

In xe_i2c_notifier(), when a new I2C adapter is added, i2c->adapter is
assigned the raw adapter pointer and i2c->work is scheduled. No reference is
taken to ensure the adapter stays alive (e.g., via i2c_get_adapter()).

The scheduled background task xe_i2c_client_work() later accesses this
pointer to instantiate a client device via i2c_new_client_device(). If the
underlying I2C adapter is unbound or removed (e.g., via a sysfs unbind of
the i2c_designware driver) between the notifier firing and the work executing,
the adapter memory will be freed. The workqueue will then dereference a
dangling pointer when trying to register the client.

[Severity: High]
This isn't a bug introduced by this patch, but can unhandled I2C adapter
removal leave dangling client pointers?

When an I2C adapter is removed (e.g., by unbinding the platform device), the
I2C core (i2c_del_adapter()) automatically unregisters and frees all
associated client devices.

The xe_i2c_notifier() does not handle BUS_NOTIFY_DEL_DEVICE to track adapter
removal, so it fails to clear the i2c->client[i] array, leaving it populated
with dangling pointers. Later, when the xe driver is unloaded and
xe_i2c_remove() runs, it unconditionally calls
i2c_unregister_device(i2c->client[i]) on the already freed client pointers,
which could lead to a double free or use-after-free.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-pdev-fwnode-ref-v1-0-88c324a1b8d2@oss.qualcomm.com?part=18