Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] host1x_device_exit() aborts traversal on first error, leaving remaining clients initialized and leaking resources. -- commit 341e3b2b98688fedc7268fa3fee081e01848b232 Author: shayderrr <[email protected]> host1x: bus: Fix missing ops null check in error teardown In host1x_device_init(), the error teardown paths do not check client->ops before dereferencing it, which can result in a null pointer dereference. This patch adds the missing client->ops check in both the teardown and teardown_late labels. Link: https://patch.msgid.link/[email protected] > diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c > index 344cc9e741c13..ffbf0ba4d602f 100644 > --- a/drivers/gpu/host1x/bus.c > +++ b/drivers/gpu/host1x/bus.c > @@ -221,7 +221,7 @@ int host1x_device_init(struct host1x_device *device) > > teardown: > list_for_each_entry_continue_reverse(client, &device->clients, list) > - if (client->ops->exit) > + if (client->ops && client->ops->exit) > client->ops->exit(client); > > /* reset client to end of list for late teardown */ > @@ -229,7 +229,7 @@ int host1x_device_init(struct host1x_device *device) > > teardown_late: > list_for_each_entry_continue_reverse(client, &device->clients, list) > - if (client->ops->late_exit) > + if (client->ops && client->ops->late_exit) > client->ops->late_exit(client); > > mutex_unlock(&device->clients_lock); This is a pre-existing issue, but I noticed a potential problem in the closely related host1x_device_exit() function while reviewing this patch: drivers/gpu/host1x/bus.c:host1x_device_exit() { ... list_for_each_entry_reverse(client, &device->clients, list) { if (client->ops && client->ops->exit) { err = client->ops->exit(client); if (err < 0) { dev_err(&device->dev, "failed to cleanup %s: %d\n", dev_name(client->dev), err); mutex_unlock(&device->clients_lock); return err; } } } ... } [Severity: High] If any client's exit() callback returns an error, this early return inside the loop skips the teardown for all remaining clients. Does this leave hardware active, leak syncpts, and leave iommu mappings dangling, which could crash the system upon further teardown? A similar issue exists a few lines lower in the late_exit() loop: drivers/gpu/host1x/bus.c:host1x_device_exit() { ... list_for_each_entry_reverse(client, &device->clients, list) { if (client->ops && client->ops->late_exit) { err = client->ops->late_exit(client); if (err < 0) { dev_err(&device->dev, "failed to late cleanup %s: %d\n", dev_name(client->dev), err); mutex_unlock(&device->clients_lock); return err; } } } ... } [Severity: High] Similarly, does this early return inside the late_exit() loop abort the traversal and leak resources for remaining clients? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
