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

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

[Fan Wu]

To: lucas.demarchi@intel.com
Cc: raag.jadav@intel.com,
    matthew.brost@intel.com,
    thomas.hellstrom@linux.intel.com,
    rodrigo.vivi@intel.com,
    airlied@gmail.com,
    simona@ffwll.ch,
    intel-xe@lists.freedesktop.org,
    dri-devel@lists.freedesktop.org,
    linux-kernel@vger.kernel.org

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;
 }

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

[Raag Jadav]

+ 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;
>  }
>