Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] drm/tiny/appletbdrm: memory leak of info buffer on appletbdrm_send_msg error - [Medium] drm/appletbdrm: coordinate underflow on resume if device dimensions shrink --
commit e4d6246ec3eb92963562077b451503c3ef907475 Author: Vasilij Markin <[email protected]> Subject: drm/appletbdrm: Add suspend and resume support The driver has no USB power management callbacks, so the USB core unbinds it before system suspend and binds it again on resume. This commit implements suspend and resume with drm_mode_config_helper_suspend() and drm_mode_config_helper_resume() so the device stays registered and the last committed state is restored on resume. > diff --git a/drivers/gpu/drm/tiny/appletbdrm.c > b/drivers/gpu/drm/tiny/appletbdrm.c > index 4297d9c1125e4..350dc598c9e7c 100644 > --- a/drivers/gpu/drm/tiny/appletbdrm.c > +++ b/drivers/gpu/drm/tiny/appletbdrm.c [ ... ] > @@ -821,6 +822,48 @@ static void appletbdrm_shutdown(struct usb_interface > *intf) > drm_atomic_helper_shutdown(&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, resume_ret; > + > + /* > + * A reset puts the device back into its initial state, where it > + * expects the same handshake as during probe. Resume the DRM side > + * even if the handshake fails, so that the state saved on suspend > + * is released. > + */ > + ret = appletbdrm_get_information(adev); [Severity: Medium] This problem wasn't introduced by this patch, but calling appletbdrm_get_information() here on every system resume can trigger a memory leak if the USB device fails to handshake correctly: drivers/gpu/drm/tiny/appletbdrm.c:appletbdrm_get_information() { ... info = kzalloc_obj(*info); if (!info) return -ENOMEM; ret = appletbdrm_send_msg(adev, APPLETBDRM_MSG_GET_INFORMATION); if (ret) return ret; /* Leaks info */ ... } Can this cause repeated memory leaks if the device frequently fails to wake? [Severity: Medium] If a buggy or malicious USB device returns smaller display dimensions during this handshake, appletbdrm_get_information() updates the device dimensions: drivers/gpu/drm/tiny/appletbdrm.c:appletbdrm_get_information() { ... adev->width = get_unaligned_le32(&info->width); adev->height = get_unaligned_le32(&info->height); ... } When the DRM core subsequently commits the suspended atomic state, it uses the old damage rectangles which are clamped to the pre-suspend mode bounds. This can cause an integer underflow in appletbdrm_flush_damage(): drivers/gpu/drm/tiny/appletbdrm.c:appletbdrm_flush_damage() { ... frame->begin_y = cpu_to_le16(adev->height - damage.x2); ... } Because damage.x2 can now be larger than the newly updated adev->height, this could underflow and send huge out-of-bounds coordinates to the USB device. Should the new dimensions be validated against the existing mode bounds before applying them? > + if (ret) { > + drm_err(drm, "Failed to get display information\n"); > + goto resume; > + } > + > + ret = appletbdrm_signal_readiness(adev); > + if (ret) > + drm_err(drm, "Failed to signal readiness\n"); > + > +resume: > + resume_ret = drm_mode_config_helper_resume(drm); > + > + return ret ? ret : resume_ret; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
