Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing drm_plane_cleanup() on error paths in display unit initialization functions leads to list corruption and use-after-free. -- commit dea10d2954fcbf7f9f6713f8f44088d76a62295e Author: Ian Forbes <[email protected]> drm/vmwgfx: Add blend mode property This commit adds blend mode properties to the primary and cursor planes to stop log spamming. It initializes the properties in vmw_du_init() for both planes if they are present. > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c [ ... ] > void vmw_du_init(struct vmw_display_unit *du) > { > vmw_vkms_crtc_init(&du->crtc); > + drm_plane_create_blend_mode_property(&du->primary, > + BIT(DRM_MODE_BLEND_PIXEL_NONE)); > + // Check that the cursor plane was actually initialized > + if (du->crtc.cursor == &du->cursor.base) > + drm_plane_create_blend_mode_property(&du->cursor.base, > + > BIT(DRM_MODE_BLEND_PREMULTI)); > } [Severity: High] This isn't a bug introduced by this patch, but I noticed a pre-existing issue regarding the cleanup of these planes on error paths during display unit initialization. In functions like vmw_ldu_init(), vmw_sou_init(), and vmw_stdu_init(), the primary and cursor planes are initialized, which links them into the DRM core plane list. If a subsequent step like drm_connector_init() fails, the code jumps to an error label and frees the display unit structure without cleaning up the planes: drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c:vmw_ldu_init() { ... ret = drm_connector_init(dev, connector, &vmw_legacy_connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL); if (ret) { DRM_ERROR("Failed to initialize connector\n"); goto err_free; } ... err_free: kfree(ldu); return ret; } Similar error paths exist in vmw_sou_init() for the sou structure, and vmw_stdu_init() for the stdu structure. Since the display unit structure containing the planes is freed with kfree() but drm_plane_cleanup() hasn't been called, could this leave dangling pointers in the DRM core plane list? If the list is later traversed during driver teardown in drm_mode_config_cleanup(), wouldn't this result in a use-after-free? Should these error paths be updated to call drm_plane_cleanup() or a similar teardown helper before freeing the memory? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
