Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/appletbdrm: Memory leak of suspend_state on reset_resume error --
commit 60e19da684ddfb4bbf66c6f0c2624ed415d96f12 Author: Vasilij Markin <[email protected]> Subject: drm/appletbdrm: Add suspend and resume support This commit introduces suspend and resume support for the appletbdrm driver. It uses drm_mode_config_helper_suspend() and drm_mode_config_helper_resume() to keep the device registered across cycles. In case the device was reset while suspended, it repeats the probe-time handshake. > diff --git a/drivers/gpu/drm/tiny/appletbdrm.c > b/drivers/gpu/drm/tiny/appletbdrm.c > index 4297d9c1125e4..024ad8bf6e1c4 100644 > --- a/drivers/gpu/drm/tiny/appletbdrm.c > +++ b/drivers/gpu/drm/tiny/appletbdrm.c [ ... ] > @@ -821,6 +822,45 @@ static void appletbdrm_shutdown(struct usb_interface > *intf) > drm_atomic_helper_shutdown(&adev->drm); > } > > +static int appletbdrm_suspend(struct usb_interface *intf, pm_message_t > message) > +{ > + struct appletbdrm_device *adev = usb_get_intfdata(intf); > + > + return drm_mode_config_helper_suspend(&adev->drm); > +} > + > +static int appletbdrm_resume(struct usb_interface *intf) > +{ > + struct appletbdrm_device *adev = usb_get_intfdata(intf); > + > + return drm_mode_config_helper_resume(&adev->drm); > +} > + > +static int appletbdrm_reset_resume(struct usb_interface *intf) > +{ > + struct appletbdrm_device *adev = usb_get_intfdata(intf); > + struct drm_device *drm = &adev->drm; > + int ret; > + > + /* > + * A reset puts the device back into its initial state, where it > + * expects the same handshake as during probe > + */ > + ret = appletbdrm_get_information(adev); > + if (ret) { > + drm_err(drm, "Failed to get display information\n"); > + return ret; [Severity: Medium] During system suspend, drm_mode_config_helper_suspend() duplicates the atomic commit state and stores it in dev->mode_config.suspend_state. If appletbdrm_get_information() fails here, we return early and bypass the call to drm_mode_config_helper_resume(). Does this leak the suspend_state and its associated DRM object references, since they never get freed or put by drm_atomic_state_put()? > + } > + > + ret = appletbdrm_signal_readiness(adev); > + if (ret) { > + drm_err(drm, "Failed to signal readiness\n"); > + return ret; [Severity: Medium] Similar to the issue above, if appletbdrm_signal_readiness() fails, does returning early without calling drm_mode_config_helper_resume() leak the atomic commit state stored in dev->mode_config.suspend_state? > + } > + > + return drm_mode_config_helper_resume(drm); > +} > + -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
