Message-ID: <sashiko-outbox-200132@kernel.org>
From: sashiko-bot@kernel.org
Subject: Re: [PATCH RESEND 54/83] drm/tests: drm_atomic_state_test: Switch
 to drm_mode_config_create_initial_state
To: "Maxime Ripard" <mripard@kernel.org>
Cc: dri-devel@lists.freedesktop.org
In-Reply-To: <20261105-drm-no-more-config-reset-v1-54-1f58d1e33380@kernel.org>
References: <20261105-drm-no-more-config-reset-v1-0-1f58d1e33380@kernel.org>
 <20261105-drm-no-more-config-reset-v1-54-1f58d1e33380@kernel.org>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Thu, 05 Nov 2026 20:46:48 +0000
Precedence: list
Reply-To: sashiko-reviews@lists.linux.dev
Sender: "dri-devel" <dri-devel-bounces@lists.freedesktop.org>

Thank you for your contribution! Sashiko AI review found 1 potential issue(=
s) to consider:
- [Medium] drm/tests: error pointer dereference in drm_atomic_commit_test c=
allers
--

commit bf780a2e8e49270edd5c0a0b1f92ae2a80dbb4a2
Author: Maxime Ripard <mripard@kernel.org>

drm/tests: drm_atomic_state_test: Switch to drm_mode_config_create_initial_=
state

This commit replaces the call to drm_mode_config_reset() in
drm_atomic_test_init_drm_components() with
drm_mode_config_create_initial_state() to initialize KMS state for the atom=
ic
commit KUnit tests.

> diff --git a/drivers/gpu/drm/tests/drm_atomic_commit_test.c b/drivers/gpu=
/drm/tests/drm_atomic_commit_test.c
> index 6ccb001f6264b..97f60ce68b3e4 100644
> --- a/drivers/gpu/drm/tests/drm_atomic_commit_test.c
> +++ b/drivers/gpu/drm/tests/drm_atomic_commit_test.c
> @@ -134,7 +134,9 @@ drm_atomic_test_init_drm_components(struct kunit *tes=
t, bool has_connectors)
>  	}
> =20
>  done:
> -	drm_mode_config_reset(drm);
> +	ret =3D drm_mode_config_create_initial_state(drm);
> +	if (ret)
> +		return ERR_PTR(ret);
> =20
>  	return priv;
>  }

[Severity: Medium]
Can returning ERR_PTR(ret) here lead to an error pointer dereference in the
callers of drm_atomic_test_init_drm_components()?

Both drm_test_check_connector_changed_modeset() and
drm_test_check_valid_clones() only check the returned pointer with
KUNIT_ASSERT_NOT_NULL():

	priv =3D drm_atomic_test_init_drm_components(test, true);
	KUNIT_ASSERT_NOT_NULL(test, priv);

If drm_mode_config_create_initial_state() returns a negative error code,
ERR_PTR(ret) will pass the KUNIT_ASSERT_NOT_NULL() check and be dereferenced
in set_up_atomic_state().

Every other failure check in drm_atomic_test_init_drm_components() uses
KUNIT_ASSERT_NOT_ERR_OR_NULL() or KUNIT_ASSERT_EQ(test, ret, 0) to abort the
test immediately. Should this check use KUNIT_ASSERT_EQ(test, ret, 0) as we=
ll,
or should the callers be updated to use KUNIT_ASSERT_NOT_ERR_OR_NULL()?

Also, this isn't a bug, but the commit subject prefix says
drm_atomic_state_test while the modified file is drm_atomic_commit_test.c.

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20261105-drm-no-mor=
e-config-reset-v1-0-1f58d1e33380@kernel.org?part=3D54