Re: [RFC PATCH] drm/sched/tests: Let the DRM scheduler manage job lifetimes

Re: [RFC PATCH] drm/sched/tests: Let the DRM scheduler manage job lifetimes

[kernel test robot]

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20260707114807.154572-1-marco.pagani@linux.dev>
References: <20260707114807.154572-1-marco.pagani@linux.dev>
TO: Marco Pagani <marco.pagani@linux.dev>

Hi Marco,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linus/master v7.2-rc6 next-20260807]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Marco-Pagani/drm-sched-tests-Let-the-DRM-scheduler-manage-job-lifetimes/20260807-105436
base:   https://gitlab.freedesktop.org/drm/misc/kernel.git drm-misc-next
patch link:    https://lore.kernel.org/r/20260707114807.154572-1-marco.pagani%40linux.dev
patch subject: [RFC PATCH] drm/sched/tests: Let the DRM scheduler manage job lifetimes
:::::: branch date: 3 days ago
:::::: commit date: 3 days ago
config: mips-randconfig-r062-20260810 (https://download.01.org/0day-ci/archive/20260810/202608101032.caQmA50M-lkp@intel.com/config)
compiler: mips-linux-gcc (GCC) 11.5.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Julia Lawall <julia.lawall@inria.fr>
| Closes: https://lore.kernel.org/r/202608101032.caQmA50M-lkp@intel.com/

cocci warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/scheduler/tests/mock_scheduler.c:139:18-21: ERROR: reference preceded by free on line 135

vim +139 drivers/gpu/drm/scheduler/tests/mock_scheduler.c

2e842124be0d553 Marco Pagani               2026-07-07  106  
5a99350794fec11 Tvrtko Ursulin             2025-03-24  107  /**
5a99350794fec11 Tvrtko Ursulin             2025-03-24  108   * drm_mock_sched_job_new - Create a new mock scheduler job
5a99350794fec11 Tvrtko Ursulin             2025-03-24  109   *
5a99350794fec11 Tvrtko Ursulin             2025-03-24  110   * @test: KUnit test owning the job
5a99350794fec11 Tvrtko Ursulin             2025-03-24  111   * @entity: Scheduler entity of the job
5a99350794fec11 Tvrtko Ursulin             2025-03-24  112   *
5a99350794fec11 Tvrtko Ursulin             2025-03-24  113   * Returns: New mock scheduler job with allocation managed by the test
5a99350794fec11 Tvrtko Ursulin             2025-03-24  114   */
5a99350794fec11 Tvrtko Ursulin             2025-03-24  115  struct drm_mock_sched_job *
5a99350794fec11 Tvrtko Ursulin             2025-03-24  116  drm_mock_sched_job_new(struct kunit *test,
5a99350794fec11 Tvrtko Ursulin             2025-03-24  117  		       struct drm_mock_sched_entity *entity)
5a99350794fec11 Tvrtko Ursulin             2025-03-24  118  {
5a99350794fec11 Tvrtko Ursulin             2025-03-24  119  	struct drm_mock_sched_job *job;
5a99350794fec11 Tvrtko Ursulin             2025-03-24  120  	int ret;
5a99350794fec11 Tvrtko Ursulin             2025-03-24  121  
2e842124be0d553 Marco Pagani               2026-07-07  122  	/* Let the DRM Scheduler manage the lifetime of the job */
2e842124be0d553 Marco Pagani               2026-07-07  123  	job = kzalloc_obj(*job, GFP_KERNEL);
5a99350794fec11 Tvrtko Ursulin             2025-03-24  124  	KUNIT_ASSERT_NOT_NULL(test, job);
5a99350794fec11 Tvrtko Ursulin             2025-03-24  125  
2e842124be0d553 Marco Pagani               2026-07-07  126  	kref_init(&job->refcount);
2e842124be0d553 Marco Pagani               2026-07-07  127  	job->test = test;
2e842124be0d553 Marco Pagani               2026-07-07  128  
5a99350794fec11 Tvrtko Ursulin             2025-03-24  129  	ret = drm_sched_job_init(&job->base,
5a99350794fec11 Tvrtko Ursulin             2025-03-24  130  				 &entity->base,
5a99350794fec11 Tvrtko Ursulin             2025-03-24  131  				 1,
2956554823cedb3 Pierre-Eric Pelloux-Prayer 2025-05-26  132  				 NULL,
2956554823cedb3 Pierre-Eric Pelloux-Prayer 2025-05-26  133  				 1);
2e842124be0d553 Marco Pagani               2026-07-07  134  	if (ret) {
2e842124be0d553 Marco Pagani               2026-07-07 @135  		kfree(job);
2e842124be0d553 Marco Pagani               2026-07-07  136  		KUNIT_ASSERT_EQ_MSG(test, ret, 0, "drm_sched_job_init failed");
2e842124be0d553 Marco Pagani               2026-07-07  137  	}
5a99350794fec11 Tvrtko Ursulin             2025-03-24  138  
5a99350794fec11 Tvrtko Ursulin             2025-03-24 @139  	init_completion(&job->done);
5a99350794fec11 Tvrtko Ursulin             2025-03-24  140  	INIT_LIST_HEAD(&job->link);
1afba39f9305fe4 Thomas Zimmermann          2025-04-07  141  	hrtimer_setup(&job->timer, drm_mock_sched_job_signal_timer,
1afba39f9305fe4 Thomas Zimmermann          2025-04-07  142  		      CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
5a99350794fec11 Tvrtko Ursulin             2025-03-24  143  
2e842124be0d553 Marco Pagani               2026-07-07  144  	ret = kunit_add_action(test, drm_mock_sched_job_cleanup_action, job);
2e842124be0d553 Marco Pagani               2026-07-07  145  	if (ret) {
2e842124be0d553 Marco Pagani               2026-07-07  146  		drm_mock_sched_job_put(job);
2e842124be0d553 Marco Pagani               2026-07-07  147  		KUNIT_ASSERT_EQ_MSG(test, ret, 0, "kunit_add_action failed");
2e842124be0d553 Marco Pagani               2026-07-07  148  	}
2e842124be0d553 Marco Pagani               2026-07-07  149  
5a99350794fec11 Tvrtko Ursulin             2025-03-24  150  	return job;
5a99350794fec11 Tvrtko Ursulin             2025-03-24  151  }
5a99350794fec11 Tvrtko Ursulin             2025-03-24  152  

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[RFC PATCH] drm/sched/tests: Let the DRM scheduler manage job lifetimes

[Marco Pagani]

Currently, the mock scheduler uses KUnit-managed memory for jobs. This ties
the job's memory lifetime to the test suite rather than the DRM scheduler's
callbacks. This does not represent real driver behavior and can lead to
potential Use-After-Free bugs in the tests.

Update the mock scheduler to let the lifetime of jobs be managed by the DRM
scheduler's asynchronous callbacks instead of KUnit managed memory. Add a
kref reference counter to track the job's lifetime between test suites and
the scheduler.

Finally, to avoid memory leaks in the event of an early test abortion,
register a cleanup KUnit action that automatically puts the reference to
the job.

Signed-off-by: Marco Pagani <marco.pagani@linux.dev>
---
 .../gpu/drm/scheduler/tests/mock_scheduler.c  | 68 ++++++++++++++-----
 drivers/gpu/drm/scheduler/tests/sched_tests.h |  6 ++
 2 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
index 14403a762335..51f81082f37a 100644
--- a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
+++ b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
@@ -96,6 +96,14 @@ drm_mock_sched_job_signal_timer(struct hrtimer *hrtimer)
 	return HRTIMER_NORESTART;
 }
 
+static void drm_mock_sched_job_cleanup_action(void *ptr)
+{
+	struct drm_mock_sched_job *job = ptr;
+
+	job->test = NULL;
+	drm_mock_sched_job_put(job);
+}
+
 /**
  * drm_mock_sched_job_new - Create a new mock scheduler job
  *
@@ -111,23 +119,34 @@ drm_mock_sched_job_new(struct kunit *test,
 	struct drm_mock_sched_job *job;
 	int ret;
 
-	job = kunit_kzalloc(test, sizeof(*job), GFP_KERNEL);
+	/* Let the DRM Scheduler manage the lifetime of the job */
+	job = kzalloc_obj(*job, GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, job);
 
+	kref_init(&job->refcount);
+	job->test = test;
+
 	ret = drm_sched_job_init(&job->base,
 				 &entity->base,
 				 1,
 				 NULL,
 				 1);
-	KUNIT_ASSERT_EQ(test, ret, 0);
-
-	job->test = test;
+	if (ret) {
+		kfree(job);
+		KUNIT_ASSERT_EQ_MSG(test, ret, 0, "drm_sched_job_init failed");
+	}
 
 	init_completion(&job->done);
 	INIT_LIST_HEAD(&job->link);
 	hrtimer_setup(&job->timer, drm_mock_sched_job_signal_timer,
 		      CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
 
+	ret = kunit_add_action(test, drm_mock_sched_job_cleanup_action, job);
+	if (ret) {
+		drm_mock_sched_job_put(job);
+		KUNIT_ASSERT_EQ_MSG(test, ret, 0, "kunit_add_action failed");
+	}
+
 	return job;
 }
 
@@ -152,7 +171,7 @@ static void drm_mock_sched_hw_fence_release(struct dma_fence *fence)
 
 	hrtimer_cancel(&job->timer);
 
-	/* Containing job is freed by the kunit framework */
+	drm_mock_sched_job_put(job);
 }
 
 static const struct dma_fence_ops drm_mock_sched_hw_fence_ops = {
@@ -173,6 +192,8 @@ static struct dma_fence *mock_sched_run_job(struct drm_sched_job *sched_job)
 		       sched->hw_timeline.context,
 		       atomic_inc_return(&sched->hw_timeline.next_seqno));
 
+	kref_get(&job->refcount);
+
 	dma_fence_get(&job->hw_fence); /* Reference for the job_list */
 
 	spin_lock_irq(&sched->lock);
@@ -200,6 +221,17 @@ static struct dma_fence *mock_sched_run_job(struct drm_sched_job *sched_job)
 	return &job->hw_fence;
 }
 
+static void mock_sched_free_job(struct drm_sched_job *sched_job)
+{
+	struct drm_mock_sched_job *job = drm_sched_job_to_mock_job(sched_job);
+
+	/* Only if the fence has been successfully initialized */
+	if (job->hw_fence.ops)
+		dma_fence_put(&job->hw_fence);
+
+	drm_mock_sched_job_put(job);
+}
+
 /*
  * Normally, drivers would take appropriate measures in this callback, such as
  * killing the entity the faulty job is associated with, resetting the hardware
@@ -232,21 +264,26 @@ mock_sched_timedout_job(struct drm_sched_job *sched_job)
 	}
 	spin_unlock_irqrestore(&sched->lock, flags);
 
-	dma_fence_put(&job->hw_fence);
-	drm_sched_job_cleanup(sched_job);
-	/* Mock job itself is freed by the kunit framework. */
+	mock_sched_free_job(sched_job);
 
 	return DRM_GPU_SCHED_STAT_RESET;
 }
 
-static void mock_sched_free_job(struct drm_sched_job *sched_job)
+static void drm_mock_sched_job_release(struct kref *ref)
 {
-	struct drm_mock_sched_job *job = drm_sched_job_to_mock_job(sched_job);
+	struct drm_mock_sched_job *job;
+
+	job = container_of(ref, struct drm_mock_sched_job, refcount);
 
-	dma_fence_put(&job->hw_fence);
-	drm_sched_job_cleanup(sched_job);
+	drm_sched_job_cleanup(&job->base);
 
-	/* Mock job itself is freed by the kunit framework. */
+	kfree(job);
+}
+
+void drm_mock_sched_job_put(struct drm_mock_sched_job *job)
+{
+	if (job)
+		kref_put(&job->refcount, drm_mock_sched_job_release);
 }
 
 static void mock_sched_cancel_job(struct drm_sched_job *sched_job)
@@ -265,10 +302,7 @@ static void mock_sched_cancel_job(struct drm_sched_job *sched_job)
 	}
 	spin_unlock_irqrestore(&sched->lock, flags);
 
-	/*
-	 * The GPU Scheduler will call drm_sched_backend_ops.free_job(), still.
-	 * Mock job itself is freed by the kunit framework.
-	 */
+	/* The GPU Scheduler will call drm_sched_backend_ops.free_job() */
 }
 
 static const struct drm_sched_backend_ops drm_mock_scheduler_ops = {
diff --git a/drivers/gpu/drm/scheduler/tests/sched_tests.h b/drivers/gpu/drm/scheduler/tests/sched_tests.h
index 553d45abd057..e4d33f0bf935 100644
--- a/drivers/gpu/drm/scheduler/tests/sched_tests.h
+++ b/drivers/gpu/drm/scheduler/tests/sched_tests.h
@@ -13,6 +13,7 @@
 #include <linux/list.h>
 #include <linux/mutex.h>
 #include <linux/types.h>
+#include <linux/kref.h>
 
 #include <drm/gpu_scheduler.h>
 
@@ -90,6 +91,7 @@ struct drm_mock_sched_entity {
  */
 struct drm_mock_sched_job {
 	struct drm_sched_job	base;
+	struct kref		refcount;
 
 	struct completion	done;
 
@@ -144,6 +146,8 @@ struct drm_mock_sched_job *
 drm_mock_sched_job_new(struct kunit *test,
 		       struct drm_mock_sched_entity *entity);
 
+void drm_mock_sched_job_put(struct drm_mock_sched_job *job);
+
 /**
  * drm_mock_sched_job_submit - Arm and submit a job in one go
  *
@@ -151,6 +155,8 @@ drm_mock_sched_job_new(struct kunit *test,
  */
 static inline void drm_mock_sched_job_submit(struct drm_mock_sched_job *job)
 {
+	kref_get(&job->refcount);
+
 	drm_sched_job_arm(&job->base);
 	drm_sched_entity_push_job(&job->base);
 }
-- 
2.55.0

Re: [RFC PATCH] drm/sched/tests: Let the DRM scheduler manage job lifetimes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Moving `drm_sched_job_cleanup()` into the job's kref release callback creates an unbreakable circular dependency that permanently leaks the job, the hardware fence, and the scheduler fence.
--

--- Patch [1]: [RFC PATCH] drm/sched/tests: Let the DRM scheduler manage job lifetimes ---
commit b37403ca6e558383820ecec4748cf84b025d3af9
Author: Marco Pagani <marco.pagani@linux.dev>

drm/sched/tests: Let the DRM scheduler manage job lifetimes

Currently, the mock scheduler uses KUnit-managed memory for jobs. This ties
the job's memory lifetime to the test suite rather than the DRM scheduler's
callbacks. Update the mock scheduler to let the lifetime of jobs be managed
by the DRM scheduler's asynchronous callbacks instead of KUnit managed memory.

> --- a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
> +++ b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
[ ... ]
> @@ -232,21 +264,26 @@ mock_sched_timedout_job(struct drm_sched_job *sched_job)
>  	}
>  	spin_unlock_irqrestore(&sched->lock, flags);
>  
> -	dma_fence_put(&job->hw_fence);
> -	drm_sched_job_cleanup(sched_job);
> -	/* Mock job itself is freed by the kunit framework. */
> +	mock_sched_free_job(sched_job);
>  
>  	return DRM_GPU_SCHED_STAT_RESET;
>  }
>  
> -static void mock_sched_free_job(struct drm_sched_job *sched_job)
> +static void drm_mock_sched_job_release(struct kref *ref)
>  {
> -	struct drm_mock_sched_job *job = drm_sched_job_to_mock_job(sched_job);
> +	struct drm_mock_sched_job *job;
> +
> +	job = container_of(ref, struct drm_mock_sched_job, refcount);
>  
> -	dma_fence_put(&job->hw_fence);
> -	drm_sched_job_cleanup(sched_job);
> +	drm_sched_job_cleanup(&job->base);

[Severity: Medium]
Does moving drm_sched_job_cleanup() into the kref release function create an
unbreakable circular dependency?

Looking at how the fences and job reference each other:

1. mock_sched_run_job() increments the job's refcount on behalf of the
hw_fence. This reference is only released in drm_mock_sched_hw_fence_release()
when the hw_fence refcount reaches zero.

2. The DRM scheduler stores the hw_fence in job->s_fence->parent, so the
s_fence holds a reference to the hw_fence.

3. The s_fence is freed by drm_sched_job_cleanup().

Because drm_sched_job_cleanup() is now only called when the job's refcount
reaches zero, the s_fence cannot be freed. This means the hw_fence is never
released, which in turn prevents it from dropping its reference to the job,
permanently leaking the job, the hw_fence, and the s_fence.

Prior to this patch, mock_sched_free_job() directly called
drm_sched_job_cleanup(), which broke the chain by freeing the s_fence before
the job was put.

>  
> -	/* Mock job itself is freed by the kunit framework. */
> +	kfree(job);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707114807.154572-1-marco.pagani@linux.dev?part=1

Re: [RFC PATCH] drm/sched/tests: Let the DRM scheduler manage job lifetimes

[Tvrtko Ursulin]

On 07/07/2026 12:48, Marco Pagani wrote:
> Currently, the mock scheduler uses KUnit-managed memory for jobs. This ties
> the job's memory lifetime to the test suite rather than the DRM scheduler's
> callbacks. This does not represent real driver behavior and can lead to
> potential Use-After-Free bugs in the tests.

Since you say use-after-free, that means something belonging to a test 
wasn't properly cleaned up, right? Could you help me out review by 
describing one example?

Idea was that tests are self contained, ie. when a tests exits it is 
guaranteed all resources belonging to it are gone. Everything else 
should be a test bug which IMO means it is fine to crash if a new tests 
gets that wrong. Just idle at the end of the test and everything should 
work.

> Update the mock scheduler to let the lifetime of jobs be managed by the DRM
> scheduler's asynchronous callbacks instead of KUnit managed memory. Add a
> kref reference counter to track the job's lifetime between test suites and
> the scheduler.
> 
> Finally, to avoid memory leaks in the event of an early test abortion,
> register a cleanup KUnit action that automatically puts the reference to
> the job.

So kunit managed memory, freed on test exit, is replaced by kunit 
managed actions, which also free on test exit?

Some random thoughts below while I wait to understand the motivation and 
problem statement better:

> Signed-off-by: Marco Pagani <marco.pagani@linux.dev>
> ---
>   .../gpu/drm/scheduler/tests/mock_scheduler.c  | 68 ++++++++++++++-----
>   drivers/gpu/drm/scheduler/tests/sched_tests.h |  6 ++
>   2 files changed, 57 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
> index 14403a762335..51f81082f37a 100644
> --- a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
> +++ b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
> @@ -96,6 +96,14 @@ drm_mock_sched_job_signal_timer(struct hrtimer *hrtimer)
>   	return HRTIMER_NORESTART;
>   }
>   
> +static void drm_mock_sched_job_cleanup_action(void *ptr)
> +{
> +	struct drm_mock_sched_job *job = ptr;
> +
> +	job->test = NULL;
> +	drm_mock_sched_job_put(job);
> +}
> +
>   /**
>    * drm_mock_sched_job_new - Create a new mock scheduler job
>    *
> @@ -111,23 +119,34 @@ drm_mock_sched_job_new(struct kunit *test,
>   	struct drm_mock_sched_job *job;
>   	int ret;
>   
> -	job = kunit_kzalloc(test, sizeof(*job), GFP_KERNEL);
> +	/* Let the DRM Scheduler manage the lifetime of the job */

However that does not match real driver behaviour either (circling back 
to the justification from the commit message). It is the drivers which 
own this memory, not the DRM scheduler. So in this case mock scheduler, 
which is a kunit test client, etc.

> +	job = kzalloc_obj(*job, GFP_KERNEL);
>   	KUNIT_ASSERT_NOT_NULL(test, job);
>   
> +	kref_init(&job->refcount);
> +	job->test = test;
> +
>   	ret = drm_sched_job_init(&job->base,
>   				 &entity->base,
>   				 1,
>   				 NULL,
>   				 1);
> -	KUNIT_ASSERT_EQ(test, ret, 0);
> -
> -	job->test = test;
> +	if (ret) {
> +		kfree(job);
> +		KUNIT_ASSERT_EQ_MSG(test, ret, 0, "drm_sched_job_init failed");
> +	}
>   
>   	init_completion(&job->done);
>   	INIT_LIST_HEAD(&job->link);
>   	hrtimer_setup(&job->timer, drm_mock_sched_job_signal_timer,
>   		      CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
>   
> +	ret = kunit_add_action(test, drm_mock_sched_job_cleanup_action, job);
> +	if (ret) {
> +		drm_mock_sched_job_put(job);
> +		KUNIT_ASSERT_EQ_MSG(test, ret, 0, "kunit_add_action failed");
> +	}
> +
>   	return job;
>   }
>   
> @@ -152,7 +171,7 @@ static void drm_mock_sched_hw_fence_release(struct dma_fence *fence)
>   
>   	hrtimer_cancel(&job->timer);
>   
> -	/* Containing job is freed by the kunit framework */
> +	drm_mock_sched_job_put(job);
>   }
>   
>   static const struct dma_fence_ops drm_mock_sched_hw_fence_ops = {
> @@ -173,6 +192,8 @@ static struct dma_fence *mock_sched_run_job(struct drm_sched_job *sched_job)
>   		       sched->hw_timeline.context,
>   		       atomic_inc_return(&sched->hw_timeline.next_seqno));
>   
> +	kref_get(&job->refcount);
> +
>   	dma_fence_get(&job->hw_fence); /* Reference for the job_list */
>   
>   	spin_lock_irq(&sched->lock);
> @@ -200,6 +221,17 @@ static struct dma_fence *mock_sched_run_job(struct drm_sched_job *sched_job)
>   	return &job->hw_fence;
>   }
>   
> +static void mock_sched_free_job(struct drm_sched_job *sched_job)
> +{

Free which is actually a reference put is non-obvious API.

> +	struct drm_mock_sched_job *job = drm_sched_job_to_mock_job(sched_job);
> +
> +	/* Only if the fence has been successfully initialized */
> +	if (job->hw_fence.ops)
> +		dma_fence_put(&job->hw_fence);
> +
> +	drm_mock_sched_job_put(job);
> +}
> +
>   /*
>    * Normally, drivers would take appropriate measures in this callback, such as
>    * killing the entity the faulty job is associated with, resetting the hardware
> @@ -232,21 +264,26 @@ mock_sched_timedout_job(struct drm_sched_job *sched_job)
>   	}
>   	spin_unlock_irqrestore(&sched->lock, flags);
>   
> -	dma_fence_put(&job->hw_fence);
> -	drm_sched_job_cleanup(sched_job);
> -	/* Mock job itself is freed by the kunit framework. */
> +	mock_sched_free_job(sched_job);
>   
>   	return DRM_GPU_SCHED_STAT_RESET;
>   }
>   
> -static void mock_sched_free_job(struct drm_sched_job *sched_job)
> +static void drm_mock_sched_job_release(struct kref *ref)
>   {
> -	struct drm_mock_sched_job *job = drm_sched_job_to_mock_job(sched_job);
> +	struct drm_mock_sched_job *job;
> +
> +	job = container_of(ref, struct drm_mock_sched_job, refcount);
>   
> -	dma_fence_put(&job->hw_fence);
> -	drm_sched_job_cleanup(sched_job);
> +	drm_sched_job_cleanup(&job->base);
>   
> -	/* Mock job itself is freed by the kunit framework. */
> +	kfree(job);
> +}
> +
> +void drm_mock_sched_job_put(struct drm_mock_sched_job *job)
> +{
> +	if (job)
> +		kref_put(&job->refcount, drm_mock_sched_job_release);
>   }
>   
>   static void mock_sched_cancel_job(struct drm_sched_job *sched_job)
> @@ -265,10 +302,7 @@ static void mock_sched_cancel_job(struct drm_sched_job *sched_job)
>   	}
>   	spin_unlock_irqrestore(&sched->lock, flags);
>   
> -	/*
> -	 * The GPU Scheduler will call drm_sched_backend_ops.free_job(), still.
> -	 * Mock job itself is freed by the kunit framework.
> -	 */
> +	/* The GPU Scheduler will call drm_sched_backend_ops.free_job() */
>   }
>   
>   static const struct drm_sched_backend_ops drm_mock_scheduler_ops = {
> diff --git a/drivers/gpu/drm/scheduler/tests/sched_tests.h b/drivers/gpu/drm/scheduler/tests/sched_tests.h
> index 553d45abd057..e4d33f0bf935 100644
> --- a/drivers/gpu/drm/scheduler/tests/sched_tests.h
> +++ b/drivers/gpu/drm/scheduler/tests/sched_tests.h
> @@ -13,6 +13,7 @@
>   #include <linux/list.h>
>   #include <linux/mutex.h>
>   #include <linux/types.h>
> +#include <linux/kref.h>
>   
>   #include <drm/gpu_scheduler.h>
>   
> @@ -90,6 +91,7 @@ struct drm_mock_sched_entity {
>    */
>   struct drm_mock_sched_job {
>   	struct drm_sched_job	base;
> +	struct kref		refcount;
>   
>   	struct completion	done;
>   
> @@ -144,6 +146,8 @@ struct drm_mock_sched_job *
>   drm_mock_sched_job_new(struct kunit *test,
>   		       struct drm_mock_sched_entity *entity);
>   
> +void drm_mock_sched_job_put(struct drm_mock_sched_job *job);
> +
>   /**
>    * drm_mock_sched_job_submit - Arm and submit a job in one go
>    *
> @@ -151,6 +155,8 @@ drm_mock_sched_job_new(struct kunit *test,
>    */
>   static inline void drm_mock_sched_job_submit(struct drm_mock_sched_job *job)
>   {
> +	kref_get(&job->refcount);
> +

This feels fragile because if someone writes a test which uses arm+push 
directly bad things will happen. What is this extra reference for versus 
the one obtained by creating a job?

Regards,

Tvrtko

>   	drm_sched_job_arm(&job->base);
>   	drm_sched_entity_push_job(&job->base);
>   }

Re: [RFC PATCH] drm/sched/tests: Let the DRM scheduler manage job lifetimes

[Marco Pagani]

On 08/07/2026 11:20, Tvrtko Ursulin wrote:
> 
> On 07/07/2026 12:48, Marco Pagani wrote:
>> Currently, the mock scheduler uses KUnit-managed memory for jobs. This ties
>> the job's memory lifetime to the test suite rather than the DRM scheduler's
>> callbacks. This does not represent real driver behavior and can lead to
>> potential Use-After-Free bugs in the tests.
> 
> Since you say use-after-free, that means something belonging to a test 
> wasn't properly cleaned up, right? Could you help me out review by 
> describing one example?

Sure, I experienced a UAF myself while writing the sched_parallel test
suite (v3->v4). The UAF happened in v3 because of a reference that the
scheduler backend keeps to the fence of the last scheduled job (in
drm_sched_run_job_work()->drm_sched_entity_pop_job()). If the job is
released before its entity, the pointer becomes dangling.

On a NR_CPUS > 20 system, this caused a UAF during the teardown phase
because jobs were freed before their entities by the KUnit actions
implicitly registered by drm_mock_sched_job_new() when allocating the
memory. To solve this in v4, I had to move KUnit release action for the
entities to ensure that entities are freed before jobs.

I also have another UAF reported by our internal CI that I haven't yet
fully investigated in drm_sched_basic:

KUNIT: drm_sched: BUG: KASAN: null-ptr-deref in drm_sched_basic_test+0x3b4/0x7c0 [drm_sched_tests]


> Idea was that tests are self contained, ie. when a tests exits it is 
> guaranteed all resources belonging to it are gone. Everything else 
> should be a test bug which IMO means it is fine to crash if a new tests 
> gets that wrong. Just idle at the end of the test and everything should 
> work.

I fully agree that test should be self-contained. However, my point is
that KUnit infrastructure should serve as a sandbox to avoid leaking
resources and not force or perturb the lifetime of test items (jobs in
this case) within the boundaries of the test.

I think the problem with the current design of the mock scheduler is
that drm_mock_sched_job_new() implicitly registers a cleanup action to
free the job's memory independently of the job's state. Even more subtly,
this approach implicitly defines a LIFO teardown order depending on when
drm_mock_sched_job_new() is called in the test.

This may confuse test developers who might be unaware of this behavior,
as they are likely to assume that the lifetime of the job will be always
guaranteed during the entire test, as I've experienced myself.


>> Update the mock scheduler to let the lifetime of jobs be managed by the DRM
>> scheduler's asynchronous callbacks instead of KUnit managed memory. Add a
>> kref reference counter to track the job's lifetime between test suites and
>> the scheduler.
>>
>> Finally, to avoid memory leaks in the event of an early test abortion,
>> register a cleanup KUnit action that automatically puts the reference to
>> the job.
> 
> So kunit managed memory, freed on test exit, is replaced by kunit 
> managed actions, which also free on test exit?

The idea is to use KUnit actions to guarantee tests against resource
leaks by dropping a reference, and let the same callback used by the
scheduler to free the memory, instead of abruptly free the memory from
right under the scheduler's nose.


> 
> Some random thoughts below while I wait to understand the motivation and 
> problem statement better:
> 
>> Signed-off-by: Marco Pagani <marco.pagani@linux.dev>
>> ---
>>   .../gpu/drm/scheduler/tests/mock_scheduler.c  | 68 ++++++++++++++-----
>>   drivers/gpu/drm/scheduler/tests/sched_tests.h |  6 ++
>>   2 files changed, 57 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
>> index 14403a762335..51f81082f37a 100644
>> --- a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
>> +++ b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
>> @@ -96,6 +96,14 @@ drm_mock_sched_job_signal_timer(struct hrtimer *hrtimer)
>>   	return HRTIMER_NORESTART;
>>   }
>>   
>> +static void drm_mock_sched_job_cleanup_action(void *ptr)
>> +{
>> +	struct drm_mock_sched_job *job = ptr;
>> +
>> +	job->test = NULL;
>> +	drm_mock_sched_job_put(job);
>> +}
>> +
>>   /**
>>    * drm_mock_sched_job_new - Create a new mock scheduler job
>>    *
>> @@ -111,23 +119,34 @@ drm_mock_sched_job_new(struct kunit *test,
>>   	struct drm_mock_sched_job *job;
>>   	int ret;
>>   
>> -	job = kunit_kzalloc(test, sizeof(*job), GFP_KERNEL);
>> +	/* Let the DRM Scheduler manage the lifetime of the job */
> 
> However that does not match real driver behaviour either (circling back 
> to the justification from the commit message). It is the drivers which 
> own this memory, not the DRM scheduler. So in this case mock scheduler, 
> which is a kunit test client, etc.

I'm not entirely following on this. Sure, it is the driver that
allocates and owns the job's memory, but it is the the scheduler backend
that manage the job's lifecycle and decides when a it is finished and
the underlying memory can be freed. To be more precise, I think it is
fair to say that when a job is submitted, the ownership is transferred
from the driver the to scheduler backend until the job is finished.


>> +	job = kzalloc_obj(*job, GFP_KERNEL);
>>   	KUNIT_ASSERT_NOT_NULL(test, job);
>>   
>> +	kref_init(&job->refcount);
>> +	job->test = test;
>> +
>>   	ret = drm_sched_job_init(&job->base,
>>   				 &entity->base,
>>   				 1,
>>   				 NULL,
>>   				 1);
>> -	KUNIT_ASSERT_EQ(test, ret, 0);
>> -
>> -	job->test = test;
>> +	if (ret) {
>> +		kfree(job);
>> +		KUNIT_ASSERT_EQ_MSG(test, ret, 0, "drm_sched_job_init failed");
>> +	}
>>   
>>   	init_completion(&job->done);
>>   	INIT_LIST_HEAD(&job->link);
>>   	hrtimer_setup(&job->timer, drm_mock_sched_job_signal_timer,
>>   		      CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
>>   
>> +	ret = kunit_add_action(test, drm_mock_sched_job_cleanup_action, job);
>> +	if (ret) {
>> +		drm_mock_sched_job_put(job);
>> +		KUNIT_ASSERT_EQ_MSG(test, ret, 0, "kunit_add_action failed");
>> +	}
>> +
>>   	return job;
>>   }
>>   
>> @@ -152,7 +171,7 @@ static void drm_mock_sched_hw_fence_release(struct dma_fence *fence)
>>   
>>   	hrtimer_cancel(&job->timer);
>>   
>> -	/* Containing job is freed by the kunit framework */
>> +	drm_mock_sched_job_put(job);
>>   }
>>   
>>   static const struct dma_fence_ops drm_mock_sched_hw_fence_ops = {
>> @@ -173,6 +192,8 @@ static struct dma_fence *mock_sched_run_job(struct drm_sched_job *sched_job)
>>   		       sched->hw_timeline.context,
>>   		       atomic_inc_return(&sched->hw_timeline.next_seqno));
>>   
>> +	kref_get(&job->refcount);
>> +
>>   	dma_fence_get(&job->hw_fence); /* Reference for the job_list */
>>   
>>   	spin_lock_irq(&sched->lock);
>> @@ -200,6 +221,17 @@ static struct dma_fence *mock_sched_run_job(struct drm_sched_job *sched_job)
>>   	return &job->hw_fence;
>>   }
>>   
>> +static void mock_sched_free_job(struct drm_sched_job *sched_job)
>> +{
> 
> Free which is actually a reference put is non-obvious API.

Uhm, this is not an API but the implementation of the .free_job method
of drm_mock_scheduler_ops. I've kept the function name since it matches
the name of backend op.


>> +	struct drm_mock_sched_job *job = drm_sched_job_to_mock_job(sched_job);
>> +
>> +	/* Only if the fence has been successfully initialized */
>> +	if (job->hw_fence.ops)
>> +		dma_fence_put(&job->hw_fence);
>> +
>> +	drm_mock_sched_job_put(job);
>> +}
>> +
>>   /*
>>    * Normally, drivers would take appropriate measures in this callback, such as
>>    * killing the entity the faulty job is associated with, resetting the hardware
>> @@ -232,21 +264,26 @@ mock_sched_timedout_job(struct drm_sched_job *sched_job)
>>   	}
>>   	spin_unlock_irqrestore(&sched->lock, flags);
>>   
>> -	dma_fence_put(&job->hw_fence);
>> -	drm_sched_job_cleanup(sched_job);
>> -	/* Mock job itself is freed by the kunit framework. */
>> +	mock_sched_free_job(sched_job);
>>   
>>   	return DRM_GPU_SCHED_STAT_RESET;
>>   }
>>   
>> -static void mock_sched_free_job(struct drm_sched_job *sched_job)
>> +static void drm_mock_sched_job_release(struct kref *ref)
>>   {
>> -	struct drm_mock_sched_job *job = drm_sched_job_to_mock_job(sched_job);
>> +	struct drm_mock_sched_job *job;
>> +
>> +	job = container_of(ref, struct drm_mock_sched_job, refcount);
>>   
>> -	dma_fence_put(&job->hw_fence);
>> -	drm_sched_job_cleanup(sched_job);
>> +	drm_sched_job_cleanup(&job->base);
>>   
>> -	/* Mock job itself is freed by the kunit framework. */
>> +	kfree(job);
>> +}
>> +
>> +void drm_mock_sched_job_put(struct drm_mock_sched_job *job)
>> +{
>> +	if (job)
>> +		kref_put(&job->refcount, drm_mock_sched_job_release);
>>   }
>>   
>>   static void mock_sched_cancel_job(struct drm_sched_job *sched_job)
>> @@ -265,10 +302,7 @@ static void mock_sched_cancel_job(struct drm_sched_job *sched_job)
>>   	}
>>   	spin_unlock_irqrestore(&sched->lock, flags);
>>   
>> -	/*
>> -	 * The GPU Scheduler will call drm_sched_backend_ops.free_job(), still.
>> -	 * Mock job itself is freed by the kunit framework.
>> -	 */
>> +	/* The GPU Scheduler will call drm_sched_backend_ops.free_job() */
>>   }
>>   
>>   static const struct drm_sched_backend_ops drm_mock_scheduler_ops = {
>> diff --git a/drivers/gpu/drm/scheduler/tests/sched_tests.h b/drivers/gpu/drm/scheduler/tests/sched_tests.h
>> index 553d45abd057..e4d33f0bf935 100644
>> --- a/drivers/gpu/drm/scheduler/tests/sched_tests.h
>> +++ b/drivers/gpu/drm/scheduler/tests/sched_tests.h
>> @@ -13,6 +13,7 @@
>>   #include <linux/list.h>
>>   #include <linux/mutex.h>
>>   #include <linux/types.h>
>> +#include <linux/kref.h>
>>   
>>   #include <drm/gpu_scheduler.h>
>>   
>> @@ -90,6 +91,7 @@ struct drm_mock_sched_entity {
>>    */
>>   struct drm_mock_sched_job {
>>   	struct drm_sched_job	base;
>> +	struct kref		refcount;
>>   
>>   	struct completion	done;
>>   
>> @@ -144,6 +146,8 @@ struct drm_mock_sched_job *
>>   drm_mock_sched_job_new(struct kunit *test,
>>   		       struct drm_mock_sched_entity *entity);
>>   
>> +void drm_mock_sched_job_put(struct drm_mock_sched_job *job);
>> +
>>   /**
>>    * drm_mock_sched_job_submit - Arm and submit a job in one go
>>    *
>> @@ -151,6 +155,8 @@ drm_mock_sched_job_new(struct kunit *test,
>>    */
>>   static inline void drm_mock_sched_job_submit(struct drm_mock_sched_job *job)
>>   {
>> +	kref_get(&job->refcount);
>> +
> 
> This feels fragile because if someone writes a test which uses arm+push 
> directly bad things will happen. What is this extra reference for versus 
> the one obtained by creating a job?

Fair point. I've consider taking the reference also before run in
mock_sched_run_job() or directly in drm_mock_sched_job_new(), but both
approaches felt suboptimal. I have to think about this. I'll come up
with a better approach for v2.


> Regards,
> 
> Tvrtko
> 
>>   	drm_sched_job_arm(&job->base);
>>   	drm_sched_entity_push_job(&job->base);
>>   }
> 

Thanks,
Marco

Re: [RFC PATCH] drm/sched/tests: Let the DRM scheduler manage job lifetimes

[Tvrtko Ursulin]

On 09/07/2026 22:53, Marco Pagani wrote:
> On 08/07/2026 11:20, Tvrtko Ursulin wrote:
>>
>> On 07/07/2026 12:48, Marco Pagani wrote:
>>> Currently, the mock scheduler uses KUnit-managed memory for jobs. This ties
>>> the job's memory lifetime to the test suite rather than the DRM scheduler's
>>> callbacks. This does not represent real driver behavior and can lead to
>>> potential Use-After-Free bugs in the tests.
>>
>> Since you say use-after-free, that means something belonging to a test
>> wasn't properly cleaned up, right? Could you help me out review by
>> describing one example?
> 
> Sure, I experienced a UAF myself while writing the sched_parallel test
> suite (v3->v4). The UAF happened in v3 because of a reference that the
> scheduler backend keeps to the fence of the last scheduled job (in
> drm_sched_run_job_work()->drm_sched_entity_pop_job()). If the job is
> released before its entity, the pointer becomes dangling.
> 
> On a NR_CPUS > 20 system, this caused a UAF during the teardown phase
> because jobs were freed before their entities by the KUnit actions
> implicitly registered by drm_mock_sched_job_new() when allocating the
> memory. To solve this in v4, I had to move KUnit release action for the
> entities to ensure that entities are freed before jobs.

Why wouldn't it work to idle the entities and the scheduler before the 
test exits? There is even kunit_suite.exit which some other tests use.

> I also have another UAF reported by our internal CI that I haven't yet
> fully investigated in drm_sched_basic:
> 
> KUNIT: drm_sched: BUG: KASAN: null-ptr-deref in drm_sched_basic_test+0x3b4/0x7c0 [drm_sched_tests]

This one is interesting. Is it maybe the dep_chain case? Perhaps prev 
fence reference needs to be taken before submit and carrier over to next 
iteration.

>> Idea was that tests are self contained, ie. when a tests exits it is
>> guaranteed all resources belonging to it are gone. Everything else
>> should be a test bug which IMO means it is fine to crash if a new tests
>> gets that wrong. Just idle at the end of the test and everything should
>> work.
> 
> I fully agree that test should be self-contained. However, my point is
> that KUnit infrastructure should serve as a sandbox to avoid leaking
> resources and not force or perturb the lifetime of test items (jobs in
> this case) within the boundaries of the test.
> 
> I think the problem with the current design of the mock scheduler is
> that drm_mock_sched_job_new() implicitly registers a cleanup action to
> free the job's memory independently of the job's state. Even more subtly,
> this approach implicitly defines a LIFO teardown order depending on when
> drm_mock_sched_job_new() is called in the test.
> 
> This may confuse test developers who might be unaware of this behavior,
> as they are likely to assume that the lifetime of the job will be always
> guaranteed during the entire test, as I've experienced myself.

Are you saying the unexpected thing is that job can be safely 
dereferenced for the duration of the test? Or that it cannot? Latter 
would be a bug somewhere AFAICT.

>>> Update the mock scheduler to let the lifetime of jobs be managed by the DRM
>>> scheduler's asynchronous callbacks instead of KUnit managed memory. Add a
>>> kref reference counter to track the job's lifetime between test suites and
>>> the scheduler.
>>>
>>> Finally, to avoid memory leaks in the event of an early test abortion,
>>> register a cleanup KUnit action that automatically puts the reference to
>>> the job.
>>
>> So kunit managed memory, freed on test exit, is replaced by kunit
>> managed actions, which also free on test exit?
> 
> The idea is to use KUnit actions to guarantee tests against resource
> leaks by dropping a reference, and let the same callback used by the
> scheduler to free the memory, instead of abruptly free the memory from
> right under the scheduler's nose.

I first want to understand exactly what the issue is. The current design 
exactly cannot free the memory under the scheduler nose unless there is 
a test bug where test exited and has left the scheduler with unprocessed 
jobs. I am not saying we cannot have reference counted jobs but, again, 
at the moment I don't understand the problem.

Regards,

Tvrtko

>> Some random thoughts below while I wait to understand the motivation and
>> problem statement better:
>>
>>> Signed-off-by: Marco Pagani <marco.pagani@linux.dev>
>>> ---
>>>    .../gpu/drm/scheduler/tests/mock_scheduler.c  | 68 ++++++++++++++-----
>>>    drivers/gpu/drm/scheduler/tests/sched_tests.h |  6 ++
>>>    2 files changed, 57 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
>>> index 14403a762335..51f81082f37a 100644
>>> --- a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
>>> +++ b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
>>> @@ -96,6 +96,14 @@ drm_mock_sched_job_signal_timer(struct hrtimer *hrtimer)
>>>    	return HRTIMER_NORESTART;
>>>    }
>>>    
>>> +static void drm_mock_sched_job_cleanup_action(void *ptr)
>>> +{
>>> +	struct drm_mock_sched_job *job = ptr;
>>> +
>>> +	job->test = NULL;
>>> +	drm_mock_sched_job_put(job);
>>> +}
>>> +
>>>    /**
>>>     * drm_mock_sched_job_new - Create a new mock scheduler job
>>>     *
>>> @@ -111,23 +119,34 @@ drm_mock_sched_job_new(struct kunit *test,
>>>    	struct drm_mock_sched_job *job;
>>>    	int ret;
>>>    
>>> -	job = kunit_kzalloc(test, sizeof(*job), GFP_KERNEL);
>>> +	/* Let the DRM Scheduler manage the lifetime of the job */
>>
>> However that does not match real driver behaviour either (circling back
>> to the justification from the commit message). It is the drivers which
>> own this memory, not the DRM scheduler. So in this case mock scheduler,
>> which is a kunit test client, etc.
> 
> I'm not entirely following on this. Sure, it is the driver that
> allocates and owns the job's memory, but it is the the scheduler backend
> that manage the job's lifecycle and decides when a it is finished and
> the underlying memory can be freed. To be more precise, I think it is
> fair to say that when a job is submitted, the ownership is transferred
> from the driver the to scheduler backend until the job is finished.
> 
> 
>>> +	job = kzalloc_obj(*job, GFP_KERNEL);
>>>    	KUNIT_ASSERT_NOT_NULL(test, job);
>>>    
>>> +	kref_init(&job->refcount);
>>> +	job->test = test;
>>> +
>>>    	ret = drm_sched_job_init(&job->base,
>>>    				 &entity->base,
>>>    				 1,
>>>    				 NULL,
>>>    				 1);
>>> -	KUNIT_ASSERT_EQ(test, ret, 0);
>>> -
>>> -	job->test = test;
>>> +	if (ret) {
>>> +		kfree(job);
>>> +		KUNIT_ASSERT_EQ_MSG(test, ret, 0, "drm_sched_job_init failed");
>>> +	}
>>>    
>>>    	init_completion(&job->done);
>>>    	INIT_LIST_HEAD(&job->link);
>>>    	hrtimer_setup(&job->timer, drm_mock_sched_job_signal_timer,
>>>    		      CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
>>>    
>>> +	ret = kunit_add_action(test, drm_mock_sched_job_cleanup_action, job);
>>> +	if (ret) {
>>> +		drm_mock_sched_job_put(job);
>>> +		KUNIT_ASSERT_EQ_MSG(test, ret, 0, "kunit_add_action failed");
>>> +	}
>>> +
>>>    	return job;
>>>    }
>>>    
>>> @@ -152,7 +171,7 @@ static void drm_mock_sched_hw_fence_release(struct dma_fence *fence)
>>>    
>>>    	hrtimer_cancel(&job->timer);
>>>    
>>> -	/* Containing job is freed by the kunit framework */
>>> +	drm_mock_sched_job_put(job);
>>>    }
>>>    
>>>    static const struct dma_fence_ops drm_mock_sched_hw_fence_ops = {
>>> @@ -173,6 +192,8 @@ static struct dma_fence *mock_sched_run_job(struct drm_sched_job *sched_job)
>>>    		       sched->hw_timeline.context,
>>>    		       atomic_inc_return(&sched->hw_timeline.next_seqno));
>>>    
>>> +	kref_get(&job->refcount);
>>> +
>>>    	dma_fence_get(&job->hw_fence); /* Reference for the job_list */
>>>    
>>>    	spin_lock_irq(&sched->lock);
>>> @@ -200,6 +221,17 @@ static struct dma_fence *mock_sched_run_job(struct drm_sched_job *sched_job)
>>>    	return &job->hw_fence;
>>>    }
>>>    
>>> +static void mock_sched_free_job(struct drm_sched_job *sched_job)
>>> +{
>>
>> Free which is actually a reference put is non-obvious API.
> 
> Uhm, this is not an API but the implementation of the .free_job method
> of drm_mock_scheduler_ops. I've kept the function name since it matches
> the name of backend op.
> 
> 
>>> +	struct drm_mock_sched_job *job = drm_sched_job_to_mock_job(sched_job);
>>> +
>>> +	/* Only if the fence has been successfully initialized */
>>> +	if (job->hw_fence.ops)
>>> +		dma_fence_put(&job->hw_fence);
>>> +
>>> +	drm_mock_sched_job_put(job);
>>> +}
>>> +
>>>    /*
>>>     * Normally, drivers would take appropriate measures in this callback, such as
>>>     * killing the entity the faulty job is associated with, resetting the hardware
>>> @@ -232,21 +264,26 @@ mock_sched_timedout_job(struct drm_sched_job *sched_job)
>>>    	}
>>>    	spin_unlock_irqrestore(&sched->lock, flags);
>>>    
>>> -	dma_fence_put(&job->hw_fence);
>>> -	drm_sched_job_cleanup(sched_job);
>>> -	/* Mock job itself is freed by the kunit framework. */
>>> +	mock_sched_free_job(sched_job);
>>>    
>>>    	return DRM_GPU_SCHED_STAT_RESET;
>>>    }
>>>    
>>> -static void mock_sched_free_job(struct drm_sched_job *sched_job)
>>> +static void drm_mock_sched_job_release(struct kref *ref)
>>>    {
>>> -	struct drm_mock_sched_job *job = drm_sched_job_to_mock_job(sched_job);
>>> +	struct drm_mock_sched_job *job;
>>> +
>>> +	job = container_of(ref, struct drm_mock_sched_job, refcount);
>>>    
>>> -	dma_fence_put(&job->hw_fence);
>>> -	drm_sched_job_cleanup(sched_job);
>>> +	drm_sched_job_cleanup(&job->base);
>>>    
>>> -	/* Mock job itself is freed by the kunit framework. */
>>> +	kfree(job);
>>> +}
>>> +
>>> +void drm_mock_sched_job_put(struct drm_mock_sched_job *job)
>>> +{
>>> +	if (job)
>>> +		kref_put(&job->refcount, drm_mock_sched_job_release);
>>>    }
>>>    
>>>    static void mock_sched_cancel_job(struct drm_sched_job *sched_job)
>>> @@ -265,10 +302,7 @@ static void mock_sched_cancel_job(struct drm_sched_job *sched_job)
>>>    	}
>>>    	spin_unlock_irqrestore(&sched->lock, flags);
>>>    
>>> -	/*
>>> -	 * The GPU Scheduler will call drm_sched_backend_ops.free_job(), still.
>>> -	 * Mock job itself is freed by the kunit framework.
>>> -	 */
>>> +	/* The GPU Scheduler will call drm_sched_backend_ops.free_job() */
>>>    }
>>>    
>>>    static const struct drm_sched_backend_ops drm_mock_scheduler_ops = {
>>> diff --git a/drivers/gpu/drm/scheduler/tests/sched_tests.h b/drivers/gpu/drm/scheduler/tests/sched_tests.h
>>> index 553d45abd057..e4d33f0bf935 100644
>>> --- a/drivers/gpu/drm/scheduler/tests/sched_tests.h
>>> +++ b/drivers/gpu/drm/scheduler/tests/sched_tests.h
>>> @@ -13,6 +13,7 @@
>>>    #include <linux/list.h>
>>>    #include <linux/mutex.h>
>>>    #include <linux/types.h>
>>> +#include <linux/kref.h>
>>>    
>>>    #include <drm/gpu_scheduler.h>
>>>    
>>> @@ -90,6 +91,7 @@ struct drm_mock_sched_entity {
>>>     */
>>>    struct drm_mock_sched_job {
>>>    	struct drm_sched_job	base;
>>> +	struct kref		refcount;
>>>    
>>>    	struct completion	done;
>>>    
>>> @@ -144,6 +146,8 @@ struct drm_mock_sched_job *
>>>    drm_mock_sched_job_new(struct kunit *test,
>>>    		       struct drm_mock_sched_entity *entity);
>>>    
>>> +void drm_mock_sched_job_put(struct drm_mock_sched_job *job);
>>> +
>>>    /**
>>>     * drm_mock_sched_job_submit - Arm and submit a job in one go
>>>     *
>>> @@ -151,6 +155,8 @@ drm_mock_sched_job_new(struct kunit *test,
>>>     */
>>>    static inline void drm_mock_sched_job_submit(struct drm_mock_sched_job *job)
>>>    {
>>> +	kref_get(&job->refcount);
>>> +
>>
>> This feels fragile because if someone writes a test which uses arm+push
>> directly bad things will happen. What is this extra reference for versus
>> the one obtained by creating a job?
> 
> Fair point. I've consider taking the reference also before run in
> mock_sched_run_job() or directly in drm_mock_sched_job_new(), but both
> approaches felt suboptimal. I have to think about this. I'll come up
> with a better approach for v2.
> 
> 
>> Regards,
>>
>> Tvrtko
>>
>>>    	drm_sched_job_arm(&job->base);
>>>    	drm_sched_entity_push_job(&job->base);
>>>    }
>>
> 
> Thanks,
> Marco

Re: [RFC PATCH] drm/sched/tests: Let the DRM scheduler manage job lifetimes

[Marco Pagani]

On 10/07/2026 15:20, Tvrtko Ursulin wrote:
> 
> On 09/07/2026 22:53, Marco Pagani wrote:
>> On 08/07/2026 11:20, Tvrtko Ursulin wrote:
>>>
>>> On 07/07/2026 12:48, Marco Pagani wrote:
>>>> Currently, the mock scheduler uses KUnit-managed memory for jobs. This ties
>>>> the job's memory lifetime to the test suite rather than the DRM scheduler's
>>>> callbacks. This does not represent real driver behavior and can lead to
>>>> potential Use-After-Free bugs in the tests.
>>>
>>> Since you say use-after-free, that means something belonging to a test
>>> wasn't properly cleaned up, right? Could you help me out review by
>>> describing one example?
>>
>> Sure, I experienced a UAF myself while writing the sched_parallel test
>> suite (v3->v4). The UAF happened in v3 because of a reference that the
>> scheduler backend keeps to the fence of the last scheduled job (in
>> drm_sched_run_job_work()->drm_sched_entity_pop_job()). If the job is
>> released before its entity, the pointer becomes dangling.
>>
>> On a NR_CPUS > 20 system, this caused a UAF during the teardown phase
>> because jobs were freed before their entities by the KUnit actions
>> implicitly registered by drm_mock_sched_job_new() when allocating the
>> memory. To solve this in v4, I had to move KUnit release action for the
>> entities to ensure that entities are freed before jobs.
> 
> Why wouldn't it work to idle the entities and the scheduler before the 
> test exits? There is even kunit_suite.exit which some other tests use.

Centralizing all teardown in kunit_suite.exit would require moving all
dynamically-allocated test objects (jobs, entities, etc.) into a
test-global struct pointed by test->priv. This would require a complex
and brittle teardown function that must handle all possible partial
initialization and test assert failures. I don't think this is the way
to go. In my understanding, KUnit actions have been introduced exactly
to handle this problem in an idiomatic and uncomplicated way.

More importantly, it wouldn't solve the teardown order problem since
KUnit executes all test-registered actions before the kunit_suite.exit
function is called. Since drm_mock_sched_job_new() always registers a
KUnit action to free the job, it will always cause jobs to be freed
before entities and the scheduler are released in kunit_suite.exit,
effectively resuscitating the (v3) UAF.

On a conceptual level, I think we have two different paradigms clashing
here: the asynchronous action-based KUnit paradigm for handling assert
failures without leaking vs. the synchronous driver teardown expected by
the scheduler backend. To be fair, I'm not 100% sure that using krefs
is the best possible approach ever to bridge these two paradigms
together, but I think it's still better than the current design.


> 
>> I also have another UAF reported by our internal CI that I haven't yet
>> fully investigated in drm_sched_basic:
>>
>> KUNIT: drm_sched: BUG: KASAN: null-ptr-deref in drm_sched_basic_test+0x3b4/0x7c0 [drm_sched_tests]
> 
> This one is interesting. Is it maybe the dep_chain case? Perhaps prev 
> fence reference needs to be taken before submit and carrier over to next 
> iteration.

Honestly, I haven't yet had time to look into it.


>>> Idea was that tests are self contained, ie. when a tests exits it is
>>> guaranteed all resources belonging to it are gone. Everything else
>>> should be a test bug which IMO means it is fine to crash if a new tests
>>> gets that wrong. Just idle at the end of the test and everything should
>>> work.
>>
>> I fully agree that test should be self-contained. However, my point is
>> that KUnit infrastructure should serve as a sandbox to avoid leaking
>> resources and not force or perturb the lifetime of test items (jobs in
>> this case) within the boundaries of the test.
>>
>> I think the problem with the current design of the mock scheduler is
>> that drm_mock_sched_job_new() implicitly registers a cleanup action to
>> free the job's memory independently of the job's state. Even more subtly,
>> this approach implicitly defines a LIFO teardown order depending on when
>> drm_mock_sched_job_new() is called in the test.
>>
>> This may confuse test developers who might be unaware of this behavior,
>> as they are likely to assume that the lifetime of the job will be always
>> guaranteed during the entire test, as I've experienced myself.
> 
> Are you saying the unexpected thing is that job can be safely 
> dereferenced for the duration of the test? Or that it cannot? Latter 
> would be a bug somewhere AFAICT.

I'm saying that relying on KUnit's implicit action queue LIFO ordering
to manage object lifetime is inherently fragile. Test writers might be
unaware of this, and even if aware, they shouldn't have to memorize the
exact allocation order of entities and jobs to avoid triggering UAF(s).


>>>> Update the mock scheduler to let the lifetime of jobs be managed by the DRM
>>>> scheduler's asynchronous callbacks instead of KUnit managed memory. Add a
>>>> kref reference counter to track the job's lifetime between test suites and
>>>> the scheduler.
>>>>
>>>> Finally, to avoid memory leaks in the event of an early test abortion,
>>>> register a cleanup KUnit action that automatically puts the reference to
>>>> the job.
>>>
>>> So kunit managed memory, freed on test exit, is replaced by kunit
>>> managed actions, which also free on test exit?
>>
>> The idea is to use KUnit actions to guarantee tests against resource
>> leaks by dropping a reference, and let the same callback used by the
>> scheduler to free the memory, instead of abruptly free the memory from
>> right under the scheduler's nose.
> 
> I first want to understand exactly what the issue is. The current design 
> exactly cannot free the memory under the scheduler nose unless there is 
> a test bug where test exited and has left the scheduler with unprocessed 
> jobs. I am not saying we cannot have reference counted jobs but, again, 
> at the moment I don't understand the problem.

I think that the current design of the mock scheduler can indeed free
test objects' memory under the scheduler backend's nose. Consider the
example discussed above: the mock scheduler is instantiated and released
in the .init and .exit functions respectively, while jobs are allocated
in the test body using drm_mock_sched_job_new(). In this rather basic
use case, jobs' memory is freed before the entities and the scheduler
are released in the .exit function.


Thanks,
Marco


> Regards,
> 
> Tvrtko
> 
>>> Some random thoughts below while I wait to understand the motivation and
>>> problem statement better:
>>>
>>>> Signed-off-by: Marco Pagani <marco.pagani@linux.dev>
>>>> ---
>>>>    .../gpu/drm/scheduler/tests/mock_scheduler.c  | 68 ++++++++++++++-----
>>>>    drivers/gpu/drm/scheduler/tests/sched_tests.h |  6 ++
>>>>    2 files changed, 57 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
>>>> index 14403a762335..51f81082f37a 100644
>>>> --- a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
>>>> +++ b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
>>>> @@ -96,6 +96,14 @@ drm_mock_sched_job_signal_timer(struct hrtimer *hrtimer)
>>>>    	return HRTIMER_NORESTART;
>>>>    }
>>>>    
>>>> +static void drm_mock_sched_job_cleanup_action(void *ptr)
>>>> +{
>>>> +	struct drm_mock_sched_job *job = ptr;
>>>> +
>>>> +	job->test = NULL;
>>>> +	drm_mock_sched_job_put(job);
>>>> +}
>>>> +
>>>>    /**
>>>>     * drm_mock_sched_job_new - Create a new mock scheduler job
>>>>     *
>>>> @@ -111,23 +119,34 @@ drm_mock_sched_job_new(struct kunit *test,
>>>>    	struct drm_mock_sched_job *job;
>>>>    	int ret;
>>>>    
>>>> -	job = kunit_kzalloc(test, sizeof(*job), GFP_KERNEL);
>>>> +	/* Let the DRM Scheduler manage the lifetime of the job */
>>>
>>> However that does not match real driver behaviour either (circling back
>>> to the justification from the commit message). It is the drivers which
>>> own this memory, not the DRM scheduler. So in this case mock scheduler,
>>> which is a kunit test client, etc.
>>
>> I'm not entirely following on this. Sure, it is the driver that
>> allocates and owns the job's memory, but it is the the scheduler backend
>> that manage the job's lifecycle and decides when a it is finished and
>> the underlying memory can be freed. To be more precise, I think it is
>> fair to say that when a job is submitted, the ownership is transferred
>> from the driver the to scheduler backend until the job is finished.
>>
>>
>>>> +	job = kzalloc_obj(*job, GFP_KERNEL);
>>>>    	KUNIT_ASSERT_NOT_NULL(test, job);
>>>>    
>>>> +	kref_init(&job->refcount);
>>>> +	job->test = test;
>>>> +
>>>>    	ret = drm_sched_job_init(&job->base,
>>>>    				 &entity->base,
>>>>    				 1,
>>>>    				 NULL,
>>>>    				 1);
>>>> -	KUNIT_ASSERT_EQ(test, ret, 0);
>>>> -
>>>> -	job->test = test;
>>>> +	if (ret) {
>>>> +		kfree(job);
>>>> +		KUNIT_ASSERT_EQ_MSG(test, ret, 0, "drm_sched_job_init failed");
>>>> +	}
>>>>    
>>>>    	init_completion(&job->done);
>>>>    	INIT_LIST_HEAD(&job->link);
>>>>    	hrtimer_setup(&job->timer, drm_mock_sched_job_signal_timer,
>>>>    		      CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
>>>>    
>>>> +	ret = kunit_add_action(test, drm_mock_sched_job_cleanup_action, job);
>>>> +	if (ret) {
>>>> +		drm_mock_sched_job_put(job);
>>>> +		KUNIT_ASSERT_EQ_MSG(test, ret, 0, "kunit_add_action failed");
>>>> +	}
>>>> +
>>>>    	return job;
>>>>    }
>>>>    
>>>> @@ -152,7 +171,7 @@ static void drm_mock_sched_hw_fence_release(struct dma_fence *fence)
>>>>    
>>>>    	hrtimer_cancel(&job->timer);
>>>>    
>>>> -	/* Containing job is freed by the kunit framework */
>>>> +	drm_mock_sched_job_put(job);
>>>>    }
>>>>    
>>>>    static const struct dma_fence_ops drm_mock_sched_hw_fence_ops = {
>>>> @@ -173,6 +192,8 @@ static struct dma_fence *mock_sched_run_job(struct drm_sched_job *sched_job)
>>>>    		       sched->hw_timeline.context,
>>>>    		       atomic_inc_return(&sched->hw_timeline.next_seqno));
>>>>    
>>>> +	kref_get(&job->refcount);
>>>> +
>>>>    	dma_fence_get(&job->hw_fence); /* Reference for the job_list */
>>>>    
>>>>    	spin_lock_irq(&sched->lock);
>>>> @@ -200,6 +221,17 @@ static struct dma_fence *mock_sched_run_job(struct drm_sched_job *sched_job)
>>>>    	return &job->hw_fence;
>>>>    }
>>>>    
>>>> +static void mock_sched_free_job(struct drm_sched_job *sched_job)
>>>> +{
>>>
>>> Free which is actually a reference put is non-obvious API.
>>
>> Uhm, this is not an API but the implementation of the .free_job method
>> of drm_mock_scheduler_ops. I've kept the function name since it matches
>> the name of backend op.
>>
>>
>>>> +	struct drm_mock_sched_job *job = drm_sched_job_to_mock_job(sched_job);
>>>> +
>>>> +	/* Only if the fence has been successfully initialized */
>>>> +	if (job->hw_fence.ops)
>>>> +		dma_fence_put(&job->hw_fence);
>>>> +
>>>> +	drm_mock_sched_job_put(job);
>>>> +}
>>>> +
>>>>    /*
>>>>     * Normally, drivers would take appropriate measures in this callback, such as
>>>>     * killing the entity the faulty job is associated with, resetting the hardware
>>>> @@ -232,21 +264,26 @@ mock_sched_timedout_job(struct drm_sched_job *sched_job)
>>>>    	}
>>>>    	spin_unlock_irqrestore(&sched->lock, flags);
>>>>    
>>>> -	dma_fence_put(&job->hw_fence);
>>>> -	drm_sched_job_cleanup(sched_job);
>>>> -	/* Mock job itself is freed by the kunit framework. */
>>>> +	mock_sched_free_job(sched_job);
>>>>    
>>>>    	return DRM_GPU_SCHED_STAT_RESET;
>>>>    }
>>>>    
>>>> -static void mock_sched_free_job(struct drm_sched_job *sched_job)
>>>> +static void drm_mock_sched_job_release(struct kref *ref)
>>>>    {
>>>> -	struct drm_mock_sched_job *job = drm_sched_job_to_mock_job(sched_job);
>>>> +	struct drm_mock_sched_job *job;
>>>> +
>>>> +	job = container_of(ref, struct drm_mock_sched_job, refcount);
>>>>    
>>>> -	dma_fence_put(&job->hw_fence);
>>>> -	drm_sched_job_cleanup(sched_job);
>>>> +	drm_sched_job_cleanup(&job->base);
>>>>    
>>>> -	/* Mock job itself is freed by the kunit framework. */
>>>> +	kfree(job);
>>>> +}
>>>> +
>>>> +void drm_mock_sched_job_put(struct drm_mock_sched_job *job)
>>>> +{
>>>> +	if (job)
>>>> +		kref_put(&job->refcount, drm_mock_sched_job_release);
>>>>    }
>>>>    
>>>>    static void mock_sched_cancel_job(struct drm_sched_job *sched_job)
>>>> @@ -265,10 +302,7 @@ static void mock_sched_cancel_job(struct drm_sched_job *sched_job)
>>>>    	}
>>>>    	spin_unlock_irqrestore(&sched->lock, flags);
>>>>    
>>>> -	/*
>>>> -	 * The GPU Scheduler will call drm_sched_backend_ops.free_job(), still.
>>>> -	 * Mock job itself is freed by the kunit framework.
>>>> -	 */
>>>> +	/* The GPU Scheduler will call drm_sched_backend_ops.free_job() */
>>>>    }
>>>>    
>>>>    static const struct drm_sched_backend_ops drm_mock_scheduler_ops = {
>>>> diff --git a/drivers/gpu/drm/scheduler/tests/sched_tests.h b/drivers/gpu/drm/scheduler/tests/sched_tests.h
>>>> index 553d45abd057..e4d33f0bf935 100644
>>>> --- a/drivers/gpu/drm/scheduler/tests/sched_tests.h
>>>> +++ b/drivers/gpu/drm/scheduler/tests/sched_tests.h
>>>> @@ -13,6 +13,7 @@
>>>>    #include <linux/list.h>
>>>>    #include <linux/mutex.h>
>>>>    #include <linux/types.h>
>>>> +#include <linux/kref.h>
>>>>    
>>>>    #include <drm/gpu_scheduler.h>
>>>>    
>>>> @@ -90,6 +91,7 @@ struct drm_mock_sched_entity {
>>>>     */
>>>>    struct drm_mock_sched_job {
>>>>    	struct drm_sched_job	base;
>>>> +	struct kref		refcount;
>>>>    
>>>>    	struct completion	done;
>>>>    
>>>> @@ -144,6 +146,8 @@ struct drm_mock_sched_job *
>>>>    drm_mock_sched_job_new(struct kunit *test,
>>>>    		       struct drm_mock_sched_entity *entity);
>>>>    
>>>> +void drm_mock_sched_job_put(struct drm_mock_sched_job *job);
>>>> +
>>>>    /**
>>>>     * drm_mock_sched_job_submit - Arm and submit a job in one go
>>>>     *
>>>> @@ -151,6 +155,8 @@ drm_mock_sched_job_new(struct kunit *test,
>>>>     */
>>>>    static inline void drm_mock_sched_job_submit(struct drm_mock_sched_job *job)
>>>>    {
>>>> +	kref_get(&job->refcount);
>>>> +
>>>
>>> This feels fragile because if someone writes a test which uses arm+push
>>> directly bad things will happen. What is this extra reference for versus
>>> the one obtained by creating a job?
>>
>> Fair point. I've consider taking the reference also before run in
>> mock_sched_run_job() or directly in drm_mock_sched_job_new(), but both
>> approaches felt suboptimal. I have to think about this. I'll come up
>> with a better approach for v2.
>>
>>
>>> Regards,
>>>
>>> Tvrtko
>>>
>>>>    	drm_sched_job_arm(&job->base);
>>>>    	drm_sched_entity_push_job(&job->base);
>>>>    }
>>>
>>
>> Thanks,
>> Marco
> 

Re: [RFC PATCH] drm/sched/tests: Let the DRM scheduler manage job lifetimes

[Tvrtko Ursulin]

On 15/07/2026 17:02, Marco Pagani wrote:

8<>

>> I first want to understand exactly what the issue is. The current design
>> exactly cannot free the memory under the scheduler nose unless there is
>> a test bug where test exited and has left the scheduler with unprocessed
>> jobs. I am not saying we cannot have reference counted jobs but, again,
>> at the moment I don't understand the problem.
> 
> I think that the current design of the mock scheduler can indeed free
> test objects' memory under the scheduler backend's nose. Consider the
> example discussed above: the mock scheduler is instantiated and released
> in the .init and .exit functions respectively, while jobs are allocated
> in the test body using drm_mock_sched_job_new(). In this rather basic
> use case, jobs' memory is freed before the entities and the scheduler
> are released in the .exit function.

So at first I thought that as long as entity is freed at each test exit, 
which includes idling, and is done by all current test cases (apart from 
the parallel one where you did it via actions) it is all fine. The 
entity->last_scheduled you mention is not a concern then since it is 
cleared etc.

However, now I think that even if entity is idled and freed before test 
exit that does nothing for the jobs not yet processed by the 
drm_sched_free_job_work()-er.

Even though I was testing with KASAN I possibly did not manage to hit 
that race. Strange but I guess possible.

Unless I am missing something with the above analysis, one options is 
along the lines of what you propose - forgo using kunit managed 
allocations for anything mock scheduler owned. Second would be to stop 
using test suite init/exit for the scheduler management and just make 
each test create own scheduler and free it as it exits.

Regardless of the option, as long as it is safe against UAFs we are I 
think good. Memory leaks on test failures are not that interesting that 
it would warrant coming up with anything more complicated.

Does that make sense? If so, question is which option is simpler? 
Probably just making tests create and destroy schedulers since then 
there is no need to deal with any memory freeing. It would be just two 
lines added to a subset of tests, the ones which rely on .init/.exit.

Regards,

Tvrtko

Re: [RFC PATCH] drm/sched/tests: Let the DRM scheduler manage job lifetimes

[Marco Pagani]

On 16/07/2026 14:14, Tvrtko Ursulin wrote:
> 
> 
> On 15/07/2026 17:02, Marco Pagani wrote:
> 
> 8<>
> 
>>> I first want to understand exactly what the issue is. The current design
>>> exactly cannot free the memory under the scheduler nose unless there is
>>> a test bug where test exited and has left the scheduler with unprocessed
>>> jobs. I am not saying we cannot have reference counted jobs but, again,
>>> at the moment I don't understand the problem.
>>
>> I think that the current design of the mock scheduler can indeed free
>> test objects' memory under the scheduler backend's nose. Consider the
>> example discussed above: the mock scheduler is instantiated and released
>> in the .init and .exit functions respectively, while jobs are allocated
>> in the test body using drm_mock_sched_job_new(). In this rather basic
>> use case, jobs' memory is freed before the entities and the scheduler
>> are released in the .exit function.
> 
> So at first I thought that as long as entity is freed at each test exit, 
> which includes idling, and is done by all current test cases (apart from 
> the parallel one where you did it via actions) it is all fine. The 
> entity->last_scheduled you mention is not a concern then since it is 
> cleared etc.
> 
> However, now I think that even if entity is idled and freed before test 
> exit that does nothing for the jobs not yet processed by the 
> drm_sched_free_job_work()-er.
> 
> Even though I was testing with KASAN I possibly did not manage to hit 
> that race. Strange but I guess possible.

It was also a surprise for me when Philipp reported a suspected UAF.
Definitely a subtle bug to catch. I managed to reproduce it by building
the kernel with KASAN and kunit.autorun enabled, and then simply booting
it with no rootfs using qemu with this slightly odd configuration:

qemu-system-x86_64 -enable-kvm -m 48G -smp cpus=18 -kernel [...]

> Unless I am missing something with the above analysis, one options is 
> along the lines of what you propose - forgo using kunit managed 
> allocations for anything mock scheduler owned. Second would be to stop 
> using test suite init/exit for the scheduler management and just make 
> each test create own scheduler and free it as it exits.

I think the second approach is still fragile. If we move
drm_mock_sched_fini() at the end of each test body, any KUNIT_ASSERT_*()
failure will cause the test to abort immediately, skipping the teardown
and leaving the scheduler in an inconsistent state that could cause any
sort of problem.

Moreover, initializing/tearing down the scheduler in the test's
init/exit functions is the most intuitive and possibly the best approach
(DRY) from a test code perspective. Asking test writers to not use the
standard init/exit functions and enforcing this limitation is a burden
that I think no one deserves to take.

> Regardless of the option, as long as it is safe against UAFs we are I 
> think good. Memory leaks on test failures are not that interesting that 
> it would warrant coming up with anything more complicated.

> Does that make sense? If so, question is which option is simpler? 
> Probably just making tests create and destroy schedulers since then 
> there is no need to deal with any memory freeing. It would be just two 
> lines added to a subset of tests, the ones which rely on .init/.exit.

I thought so too when developing the drm_gem_shmem_test some time ago,
but then I changed my mind after dealing with downstream CI. A single
test leaking some memory is not optimal, but not even a big deal per se.
However, consider hundreds of KUnit tests running periodically on a CI.
If all failing tests did not care about leaking memory, they could
seriously bog down the system. I think it's responsible to have tests
that do not leak memory when they fail.

I'll see if I can come up with a cleaner version of the kref approach,
taking into account your recommendations and Sashiko's bug report.

Thank you for your suggestions,
Marco
 
> Regards,
> 
> Tvrtko
>