Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Low] Missing `inline` keyword on `to_aspeed_gfx` in a header file. Pre-existing issues: - [Critical] Direct dereference of `obj->state` inside atomic helpers leads to potential use-after-free and NULL pointer dereferences. -- --- Patch [2]: [PATCH v2 2/8] drm/aspeed: replace struct drm_simple_display_pipe with regular atomic helpers --- (Note: The format of this report is altered to a free-form summary due to automated recitation restrictions. Code snippets are not quoted directly.) commit d1e92d6b727d3655d12a72430c723784264108f5 Author: Ze Huang <[email protected]> Subject: drm/aspeed: replace struct drm_simple_display_pipe with regular atomic helpers This commit replaces the simple display pipe with explicit plane, CRTC, and encoder objects in the aspeed DRM driver. It moves callbacks to their respective atomic helpers and shifts page-flip event handling to the CRTC atomic flush path. Issue 1: Missing inline keyword File: drivers/gpu/drm/aspeed/aspeed_gfx.h Function: to_aspeed_gfx [Severity: Low] Does this function need the inline keyword? The to_aspeed_gfx function is defined as static in a header file without being marked inline. Could this cause duplicate unreferenced copies of the function to be emitted in every translation unit that includes the header, potentially leading to binary bloat or compiler warnings when compiled with -Werror? Issue 2: Direct state dereference in atomic context File: drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c Functions: aspeed_gfx_set_pixel_fmt and aspeed_gfx_crtc_mode_set_nofb [Severity: Critical] This is a pre-existing issue, but is it safe to directly dereference the object state pointers here? In aspeed_gfx_set_pixel_fmt, the code directly accesses priv->plane.state->fb->format->format. Similarly, in aspeed_gfx_crtc_mode_set_nofb, it directly accesses priv->crtc.state->adjusted_mode. Since this is accessed during atomic commits, if a second non-blocking commit disables the plane before the first commit reaches its atomic enable execution, the plane state could be updated to a new state with a NULL framebuffer. When the first commit's worker eventually reads the state, could this lead to a NULL pointer dereference and a kernel crash? Would it be safer to pass the specific plane and CRTC states down from the atomic helpers instead of reading them directly from the objects? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260716-drm-simple-kms-removal-v2-0-1133a8fc3...@oss.qualcomm.com?part=2
