Re: [PATCH v2 14/19] drm/xe/i2c: use platform_device_set_fwnode()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing cancel_work_sync() in driver teardown and error paths allows a pending background work to execute after the 'xe_i2c' structure is freed.
--

commit 1ceae3ee709a0fb69c698acf7a00cab2a5e1829c
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 is a pre-existing issue, but I noticed a potential use-after-free
involving the background work item in this file.

If platform_device_add() (called just below this block) triggers the bus
notifier but ultimately fails, the asynchronously scheduled i2c->work remains
pending.

When xe_i2c_probe() handles the error by unregistering the notifier and
returning, devres immediately frees the i2c structure. Later, when the work
executes, could it write to i2c->client[0] in freed memory?

The xe_i2c_probe() error path currently does this:

err_unregister_notifier:
	bus_unregister_notifier(&i2c_bus_type, &i2c->bus_notifier);

	return ret;

A similar scenario appears to exist in the standard teardown path, as
xe_i2c_remove() does not cancel the pending work:

static void xe_i2c_remove(void *arg)
{
	...
	bus_unregister_notifier(&i2c_bus_type, &i2c->bus_notifier);
	xe_i2c_unregister_adapter(i2c);
	xe_i2c_remove_irq(i2c);
}

Should cancel_work_sync(&i2c->work) be added to both the probe error path
and the remove function to ensure the work item doesn't execute after the
structure is freed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-pdev-fwnode-ref-v2-0-8abe2513f96e@oss.qualcomm.com?part=14