All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/sched: Create a fake device for KUnit tests
@ 2026-09-03 12:15 oushixiong1025
  2026-09-03 12:26 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: oushixiong1025 @ 2026-09-03 12:15 UTC (permalink / raw)
  To: Matthew Brost
  Cc: Danilo Krummrich, Philipp Stanner, Christian König,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, dri-devel, linux-kernel, Shixiong Ou, stable

From: Shixiong Ou <oushixiong@kylinos.cn>

The DRM scheduler KUnit tests pass NULL for the dev field in
drm_sched_init_args, which NULL-pointer dereferences in the drm_sched_job
trace event via dev_name() on sched->dev.

Give the mock scheduler a device with kunit_device_register(), which is
also cleaned up at test exit. A per-function counter keeps the device
names unique, since some tests create several mock schedulers.

Fixes: 5a99350794fe ("drm/sched: Add scheduler unit testing infrastructure and some basic tests")
Cc: stable@vger.kernel.org
Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>

---
v1->v2:
  - Switch from faux_device_create() to kunit_device_register(), which also
    cleans the device up automatically at test exit (Maxime Ripard)
  - Build the device name on top of args.name (Philipp Stanner)
  - Make the instance counter a static unsigned int local to
    drm_mock_sched_new() (Philipp Stanner)
  - Add a Fixes: tag and Cc: stable for the NULL dev dereference

 drivers/gpu/drm/scheduler/tests/mock_scheduler.c | 11 ++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
index 8e9ae7d980eb..12dc61f56192 100644
--- a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
+++ b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2025 Valve Corporation */
 
+#include <kunit/device.h>
+
 #include "sched_tests.h"
 
 /*
@@ -288,6 +290,7 @@ static const struct drm_sched_backend_ops drm_mock_scheduler_ops = {
  */
 struct drm_mock_scheduler *drm_mock_sched_new(struct kunit *test, long timeout)
 {
+	static unsigned int instance;
 	struct drm_sched_init_args args = {
 		.ops		= &drm_mock_scheduler_ops,
 		.num_rqs	= DRM_SCHED_PRIORITY_COUNT,
@@ -297,11 +300,19 @@ struct drm_mock_scheduler *drm_mock_sched_new(struct kunit *test, long timeout)
 		.name		= "drm-mock-scheduler",
 	};
 	struct drm_mock_scheduler *sched;
+	struct device *dev;
+	char name[64];
 	int ret;
 
 	sched = kunit_kzalloc(test, sizeof(*sched), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, sched);
 
+	snprintf(name, sizeof(name), "%s-%u", args.name, ++instance);
+	dev = kunit_device_register(test, name);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
+
+	args.dev = dev;
+
 	ret = drm_sched_init(&sched->base, &args);
 	KUNIT_ASSERT_EQ(test, ret, 0);
 
-- 
2.43.0

No virus found
		Checked by Hillstone Network AntiVirus


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] drm/sched: Create a fake device for KUnit tests
  2026-09-03 12:15 [PATCH v2] drm/sched: Create a fake device for KUnit tests oushixiong1025
@ 2026-09-03 12:26 ` sashiko-bot
  2026-09-03 12:38 ` Maxime Ripard
  2026-09-04  7:02 ` Philipp Stanner
  2 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-09-03 12:26 UTC (permalink / raw)
  To: oushixiong1025; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Mock scheduler workqueues are leaked and cause Use-After-Free on KUnit assertion failures.
--

commit cd473696df99f11e05b48a9d612fa14552094751
Author: Shixiong Ou <oushixiong@kylinos.cn>

drm/sched: Create a fake device for KUnit tests

This commit addresses a NULL-pointer dereference in the drm_sched_job trace
event by registering a mock device using kunit_device_register() and
assigning it to the scheduler arguments during test initialization.

> diff --git a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
> index 8e9ae7d980eb2..12dc61f56192e 100644
> --- a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
> +++ b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
[ ... ]
> @@ -297,11 +300,19 @@ struct drm_mock_scheduler *drm_mock_sched_new(struct kunit *test, long timeout)
>  		.name		= "drm-mock-scheduler",
>  	};
>  	struct drm_mock_scheduler *sched;
> +	struct device *dev;
> +	char name[64];
>  	int ret;
>  
>  	sched = kunit_kzalloc(test, sizeof(*sched), GFP_KERNEL);
>  	KUNIT_ASSERT_NOT_NULL(test, sched);
>  
> +	snprintf(name, sizeof(name), "%s-%u", args.name, ++instance);
> +	dev = kunit_device_register(test, name);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
> +
> +	args.dev = dev;
> +
>  	ret = drm_sched_init(&sched->base, &args);

[Severity: High]
This is a pre-existing issue, but does this allocation structure leave the
scheduler's submit workqueue vulnerable to a use-after-free on test failures?

In drm_sched_init(), a new submit_wq workqueue is created since
args->submit_wq is NULL. This workqueue is not managed by KUnit. If a test
encounters an assertion failure later (for example, hitting
KUNIT_ASSERT_TRUE(test, done) in drm_sched_basic_submit()), the KUnit
test thread is immediately aborted, bypassing manual cleanup routines like
drm_mock_sched_fini().

Because sched and sched->dev are managed by KUnit (via kunit_kzalloc
and kunit_device_register), they will be automatically freed upon test
abort. However, the unmanaged submit_wq remains active, and any pending
work items will execute and access the freed sched and sched->dev
memory, potentially causing a kernel crash.

Could a KUnit-managed cleanup hook like kunit_add_action() be used
after drm_sched_init() to ensure the workqueue is safely destroyed even if
the test thread aborts?

>  	KUNIT_ASSERT_EQ(test, ret, 0);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903121558.782854-1-oushixiong1025@163.com?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] drm/sched: Create a fake device for KUnit tests
  2026-09-03 12:15 [PATCH v2] drm/sched: Create a fake device for KUnit tests oushixiong1025
  2026-09-03 12:26 ` sashiko-bot
@ 2026-09-03 12:38 ` Maxime Ripard
  2026-09-04  7:02 ` Philipp Stanner
  2 siblings, 0 replies; 5+ messages in thread
From: Maxime Ripard @ 2026-09-03 12:38 UTC (permalink / raw)
  To: oushixiong1025
  Cc: dri-devel, linux-kernel, stable, Christian König,
	Danilo Krummrich, David Airlie, Maarten Lankhorst, Matthew Brost,
	Maxime Ripard, Philipp Stanner, Shixiong Ou, Simona Vetter,
	Thomas Zimmermann

On Thu, 3 Sep 2026 20:15:58 +0800, oushixiong1025@163.com wrote:
> From: Shixiong Ou <oushixiong@kylinos.cn>
> 
> The DRM scheduler KUnit tests pass NULL for the dev field in
> drm_sched_init_args, which NULL-pointer dereferences in the drm_sched_job
> trace event via dev_name() on sched->dev.
> 
> [ ... ]

Acked-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] drm/sched: Create a fake device for KUnit tests
  2026-09-03 12:15 [PATCH v2] drm/sched: Create a fake device for KUnit tests oushixiong1025
  2026-09-03 12:26 ` sashiko-bot
  2026-09-03 12:38 ` Maxime Ripard
@ 2026-09-04  7:02 ` Philipp Stanner
  2026-09-08  5:58   ` Shixiong Ou
  2 siblings, 1 reply; 5+ messages in thread
From: Philipp Stanner @ 2026-09-04  7:02 UTC (permalink / raw)
  To: oushixiong1025, Matthew Brost
  Cc: Danilo Krummrich, Philipp Stanner, Christian König,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, dri-devel, linux-kernel, Shixiong Ou, stable

On Thu, 2026-09-03 at 20:15 +0800, oushixiong1025@163.com wrote:
> 

[…]

> +
>  #include "sched_tests.h"
>  
>  /*
> @@ -288,6 +290,7 @@ static const struct drm_sched_backend_ops drm_mock_scheduler_ops = {
>   */
>  struct drm_mock_scheduler *drm_mock_sched_new(struct kunit *test, long timeout)
>  {
> +	static unsigned int instance;

I know statics are initialized to 0 automatically, but writing it out
explicitly is probably advantageous for readability, especially since
we're incrementing.

>  	struct drm_sched_init_args args = {
>  		.ops		= &drm_mock_scheduler_ops,
>  		.num_rqs	= DRM_SCHED_PRIORITY_COUNT,
> @@ -297,11 +300,19 @@ struct drm_mock_scheduler *drm_mock_sched_new(struct kunit *test, long timeout)
>  		.name		= "drm-mock-scheduler",
>  	};
>  	struct drm_mock_scheduler *sched;
> +	struct device *dev;
> +	char name[64];
>  	int ret;
>  
>  	sched = kunit_kzalloc(test, sizeof(*sched), GFP_KERNEL);
>  	KUNIT_ASSERT_NOT_NULL(test, sched);
>  
> +	snprintf(name, sizeof(name), "%s-%u", args.name, ++instance);

Any particular reason why you begin with index 1?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] drm/sched: Create a fake device for KUnit tests
  2026-09-04  7:02 ` Philipp Stanner
@ 2026-09-08  5:58   ` Shixiong Ou
  0 siblings, 0 replies; 5+ messages in thread
From: Shixiong Ou @ 2026-09-08  5:58 UTC (permalink / raw)
  To: phasta, Matthew Brost
  Cc: Danilo Krummrich, Christian König, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	dri-devel, linux-kernel, Shixiong Ou, stable

Hi,

First of all, thanks for the review and the suggestions.
I had already sent v3 by the time your full comments came in -- here is
the v3 patch for reference:  https://lkml.org/lkml/2026/9/4/547

Both points will be addressed in v4 later.


On 2026/9/4 15:02, Philipp Stanner wrote:
> On Thu, 2026-09-03 at 20:15 +0800, oushixiong1025@163.com wrote:
> […]
>
>> +
>>   #include "sched_tests.h"
>>   
>>   /*
>> @@ -288,6 +290,7 @@ static const struct drm_sched_backend_ops drm_mock_scheduler_ops = {
>>    */
>>   struct drm_mock_scheduler *drm_mock_sched_new(struct kunit *test, long timeout)
>>   {
>> +	static unsigned int instance;
> I know statics are initialized to 0 automatically, but writing it out
> explicitly is probably advantageous for readability, especially since
> we're incrementing.

One note: this trips checkpatch's "do not initialise statics to 0"
error, so I followed your suggestion over the checkpatch rule.


>>   	struct drm_sched_init_args args = {
>>   		.ops		= &drm_mock_scheduler_ops,
>>   		.num_rqs	= DRM_SCHED_PRIORITY_COUNT,
>> @@ -297,11 +300,19 @@ struct drm_mock_scheduler *drm_mock_sched_new(struct kunit *test, long timeout)
>>   		.name		= "drm-mock-scheduler",
>>   	};
>>   	struct drm_mock_scheduler *sched;
>> +	struct device *dev;
>> +	char name[64];
>>   	int ret;
>>   
>>   	sched = kunit_kzalloc(test, sizeof(*sched), GFP_KERNEL);
>>   	KUNIT_ASSERT_NOT_NULL(test, sched);
>>   
>> +	snprintf(name, sizeof(name), "%s-%u", args.name, ++instance);
> Any particular reason why you begin with index 1?

No particular reason -- switched to instance++ so the numbering
starts at 0.

The updated v4 patch will be send later.
Thanks again for your time and for catching these points.

Best regards,
Shixiong Ou



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-08  5:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-03 12:15 [PATCH v2] drm/sched: Create a fake device for KUnit tests oushixiong1025
2026-09-03 12:26 ` sashiko-bot
2026-09-03 12:38 ` Maxime Ripard
2026-09-04  7:02 ` Philipp Stanner
2026-09-08  5:58   ` Shixiong Ou

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.