Re: [PATCH 2/2] drm/amd/display: Add KUnit tests for the eDP/LVDS native mode fallback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/amd/display: uninitialized stack memory read in drm_mode_copy()
--

commit 852bbf13f64b2d512665604270ff18f1f920af2e
Author: Hugo Sales <hugo@hsal.es>

drm/amd/display: Add KUnit tests for the eDP/LVDS native mode fallback

Leave only a scaled mode in connector->modes and put the larger native
mode in the encoder, as a size-limited probe does. On eDP and LVDS,
create_stream_for_sink() must build a stream with the native timing and
the scaled mode as its source size. On HDMI it must keep the first mode
in the list as the timing.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c
> index 56fd133bc4056..55e2c38499f4a 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c
> @@ -4547,6 +4547,115 @@ static void dm_test_create_stream_hdmi_vsif(struct kunit *test)
[ ... ]
> +static struct dc_stream_state *
> +dm_test_native_mode_pruned_stream(struct kunit *test, int connector_type)
> +{
[ ... ]
> +	stream = create_stream_for_sink(&ctx->aconnector->base, ctx->mode,
> +					ctx->dm_state, NULL, 8,
> +					PIXEL_ENCODING_RGB, false);

[Severity: Medium]
Does this call to create_stream_for_sink() trigger a read of uninitialized
stack memory?

In create_stream_for_sink(), the variable native_mode is declared on the
stack without initialization:

amdgpu_dm_connector.c:create_stream_for_sink() {
	...
	struct drm_display_mode native_mode;
	...
	if (!native_mode_found && aconnector && ...) {
		...
		drm_mode_copy(&native_mode, &to_amdgpu_encoder(encoder)->native_mode);
		...
	}
}

The drm_mode_copy() function begins by saving the destination's list head:

drm_modes.c:drm_mode_copy() {
	struct list_head head = dst->head;
	...
}

Because native_mode is uninitialized, this reads uninitialized stack memory,
which could trigger KMSAN panics on systems with memory sanitizers enabled.
Should native_mode be zero-initialized or initialized using drm_mode_init()
first?

> +
> +	drm_edid_free(ctx->aconnector->drm_edid);
> +	KUNIT_ASSERT_NOT_NULL(test, stream);
> +
> +	return stream;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014223041.171317-1-hugo@hsal.es?part=2