Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] sysfb: kill-switch bypass when parent device query returns an error --
commit cea5d6870e1a5b4277264d29fe9056b6df3310b7 Author: Wentao Liang <[email protected]> firmware/sysfb: Fix device reference count leak in sysfb_disable() This commit addresses a device reference count leak in sysfb_disable(). It adds a check to ensure the parent device is not an error pointer and releases the reference using put_device() before returning. > diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c > --- a/drivers/firmware/sysfb.c > +++ b/drivers/firmware/sysfb.c > @@ -71,9 +71,12 @@ void sysfb_disable(struct device *dev) > > mutex_lock(&disable_lock); > parent = sysfb_parent_dev(si); > - if (!dev || !parent || dev == parent) { > - sysfb_unregister(); > - disabled = true; > + if (!IS_ERR(parent)) { [Severity: High] Will wrapping the unregistration logic in !IS_ERR(parent) bypass the global kill-switch when the parent device query returns an error? Graphics drivers use sysfb_disable(NULL) as a global kill-switch to unconditionally disable the generic firmware framebuffer before native DRM drivers take over. sysfb_parent_dev() can return an ERR_PTR (like -ENODEV) if the PCI device exists but its memory decoding is not enabled. If this happens during early boot or driver probe, this new check skips the block entirely, silently bypassing the kill-switch and leaving the disabled flag as false. If a built-in native DRM driver disables sysfb and subsequently enables PCI memory decoding, could a later execution of sysfb_init() see disabled == false and incorrectly register the generic framebuffer on top of the already-running native driver? This concurrent access could lead to resource conflicts or kernel panics. > + if (!dev || !parent || dev == parent) { > + sysfb_unregister(); > + disabled = true; > + } > + put_device(parent); > } > mutex_unlock(&disable_lock); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
