> Should native_mode be zero-initialized or initialized using drm_mode_init() > first?
Thanks for the catch. I'll switch to drm_mode_init(), like the mode copy a few lines above, in v2 > On 10/14/2026 11:39 PM WEST [email protected] wrote: > > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [Medium] drm: uninitialized stack memory read in create_stream_for_sink() > -- > > commit c9acc168fa3f685b9a0bbaf87f6642a89b782aef > Author: Hugo Sales <[email protected]> > drm/amd/display: Fall back to the native mode, not the first, on eDP/LVDS > > This commit updates create_stream_for_sink() to fall back to the native > mode stored in the encoder when the preferred mode is no longer present > in the connector's mode list. This is limited to eDP and LVDS panels where > amdgpu handles scaling automatically. > > Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5983 > > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c > > index 428a5f9377c62..a1dc3e28764cf 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c > > @@ -1470,6 +1472,7 @@ create_stream_for_sink(struct drm_connector > > *connector, > > struct dc_stream_state *stream = NULL; > > struct drm_display_mode mode; > > struct drm_display_mode saved_mode; > > + struct drm_display_mode native_mode; > > struct drm_display_mode *freesync_mode = NULL; > > bool native_mode_found = false; > > bool recalculate_timing = false; > > [ ... ] > > > @@ -1529,6 +1532,31 @@ create_stream_for_sink(struct drm_connector > > *connector, > > break; > > } > > } > > + > > + /* > > + * After a probe with size limits smaller than the preferred mode, > > + * connector->modes no longer has that mode, and the first mode left > > + * would become the stream timing. fbdev emulation probes with limits > > + * that video= can set below the native size. The encoder has a copy of > > + * the preferred mode from amdgpu_dm_get_native_mode(), made on every > > + * probe before the limits apply, so use that copy. Only do this for eDP > > + * and LVDS panels, which take only their native timing. On other > > + * connectors mode_valid can also reject the preferred mode, and then > > + * the first valid mode is the right one > > + */ > > + if (!native_mode_found && aconnector && !aconnector->mst_root && > > aconnector->drm_edid && > > + (connector->connector_type == DRM_MODE_CONNECTOR_eDP || > > + connector->connector_type == DRM_MODE_CONNECTOR_LVDS)) { > > + struct drm_encoder *encoder = > > amdgpu_dm_connector_to_encoder(connector); > > + > > + if (encoder && to_amdgpu_encoder(encoder)->native_mode.clock) { > > + drm_mode_copy(&native_mode, > > &to_amdgpu_encoder(encoder)->native_mode); > > [Severity: Medium] > Does this result in an uninitialized stack memory read? > > The native_mode variable is declared on the stack without initialization. > When passed to drm_mode_copy(), that function starts by attempting to > preserve the existing list node pointers of the destination: > > void drm_mode_copy(struct drm_display_mode *dst, > const struct drm_display_mode *src) > { > struct list_head head = dst->head; > > *dst = *src; > dst->head = head; > } > > Because native_mode is uninitialized, reading dst->head accesses garbage > data. While this won't corrupt an active list (since the garbage is written > back to a local variable that is never linked), it still triggers undefined > behavior and can cause Kernel Memory Sanitizer (KMSAN) warnings. > > Could we initialize native_mode when it is declared to avoid this? > For example: struct drm_display_mode native_mode = { 0 }; > > > + drm_mode_set_crtcinfo(&native_mode, > > CRTC_INTERLACE_HALVE_V); > > + preferred_mode = &native_mode; > > + native_mode_found = true; > > + } > > + } > > -- > Sashiko AI review ยท > https://sashiko.dev/#/patchset/[email protected]?part=1
