Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing cancel_work_sync() in the xe_i2c teardown path leads to a Use-After-Free of the device structure. -- commit 1fdde54b64291aceacb95c2a267bee6b6609b500 Author: Bartosz Golaszewski <[email protected]> drm/xe/i2c: use platform_device_set_fwnode() This commit encapsulates the assignment of the firmware node for dynamically allocated platform devices with the provided helper ahead of reworking the reference counting logic. > 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 does the teardown path for this adapter need a cancel_work_sync() call? The I2C adapter registration during xe_i2c_probe() triggers a BUS_NOTIFY_ADD_DEVICE notification, which schedules i2c->work. If the device is unbound before this background work completes, xe_i2c_remove() is called: static void xe_i2c_remove(void *data) { struct xe_i2c *i2c = data; unsigned int i; for (i = 0; i < XE_I2C_MAX_CLIENTS; i++) i2c_unregister_device(i2c->client[i]); bus_unregister_notifier(&i2c_bus_type, &i2c->bus_notifier); xe_i2c_unregister_adapter(i2c); xe_i2c_remove_irq(i2c); } Since xe_i2c_remove() unregisters the notifier and adapter but fails to call cancel_work_sync(&i2c->work), could the scheduled work execute after the struct xe_i2c memory is freed? This appears to leave a window for a use-after-free when unbinding the driver, as the delayed work might dereference the freed i2c pointer. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=15
