Re: [PATCH v3] drm/xe/i2c: cancel the client work on remove

[Raag Jadav <raag.jadav@intel.com>]

+ 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/20260912085932.101598-1-fanwu01@zju.edu.cn/
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:gpt-5.6
> Co-developed-by: Song Li <songl@zju.edu.cn>
> Signed-off-by: Song Li <songl@zju.edu.cn>
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
> ---
> 
> 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;
>  }
>