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 <[email protected]> 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/[email protected]?part=14
