On Thu Jul 16, 2026 at 5:27 PM CST, sashiko-bot wrote: > 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?
Will do, here and other places in series > > 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? As noted in my reply to patch 5, I'll access the state with atomic helpers instead.
