+ Heikki to comment on this.
On Mon, Sep 28, 2026 at 01:30:30AM +0000, Fan Wu wrote:
> xe_i2c_notifier() stores the DesignWare adapter in i2c->adapter and
> schedules i2c->work when the adapter is registered under the xe I2C
> platform device, and xe_i2c_client_work() then instantiates the AMC
> client device on that adapter.
>
> xe_i2c_remove() tears down the AMC, unregisters the client devices,
> the bus notifier and the adapter platform device, but it never drains
> i2c->work. A work item that is still queued or running when the
> adapter is unregistered dereferences i2c->adapter in
> i2c_new_client_device() after platform_device_unregister() has
> released the adapter. The work item is also embedded in the
> devm-allocated struct xe_i2c, so a work item still queued after the
> drm device devm unwind frees that allocation runs its callback on
> freed memory.
>
> The bus notifier is the only thing that schedules this work, and it is
> unregistered after the client devices. An instance that is still queued
> when the teardown runs can therefore write
> i2c->client[XE_I2C_CLIENT_AMC] while the loop is unregistering and
> clearing the same array, and an AMC client it instantiates late is only
> cleaned up by the adapter's own child sweep in i2c_del_adapter().
>
> Move bus_unregister_notifier() in front of the client teardown loop
> and cancel the work right after it, so no new instance can be
> scheduled and a queued instance is drained before the client array is
> touched. A running instance still finds a live adapter, since the
> adapter is unregistered later.
>
> This issue was found by an in-house static analysis tool.
>
> Fixes: f0e53aadd702 ("drm/xe: Support for I2C attached MCUs")
> Link:
> https://lore.kernel.org/intel-xe/[email protected]/
> Cc: [email protected]
> Assisted-by: Codex:gpt-5.6
> Co-developed-by: Song Li <[email protected]>
> Signed-off-by: Song Li <[email protected]>
> Signed-off-by: Fan Wu <[email protected]>
> ---
>
> Changes in v3:
> - rebase onto drm-xe-next, after "drm/xe/i2c: Disable IRQ on unbind"
> - discussion: Link: above points at the v2 thread
> - unregister the bus notifier before the client teardown loop and
> cancel the work before the loop as well: v2 cancelled the work only
> after the loop, so an event arriving during the loop could still
> schedule the work to race with the array teardown and leak a freshly
> instantiated AMC client
> drivers/gpu/drm/xe/xe_i2c.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
> index f4f3819..b82caaf 100644
> --- a/drivers/gpu/drm/xe/xe_i2c.c
> +++ b/drivers/gpu/drm/xe/xe_i2c.c
> @@ -324,12 +324,15 @@ static void xe_i2c_remove(void *data)
> xe_i2c_irq_reset(xe);
> xe_amc_exit(i2c);
>
> + /* Stop the notifier from arming the client work before teardown. */
> + bus_unregister_notifier(&i2c_bus_type, &i2c->bus_notifier);
> + cancel_work_sync(&i2c->work);
> +
> for (i = 0; i < XE_I2C_MAX_CLIENTS; i++) {
> i2c_unregister_device(i2c->client[i]);
> i2c->client[i] = NULL;
> }
>
> - bus_unregister_notifier(&i2c_bus_type, &i2c->bus_notifier);
> xe_i2c_unregister_adapter(i2c);
> xe->i2c = NULL;
> }
>
