* [PATCH] fix: drm/ttm/tests: use KUNIT_ASSERT_EQ for critical error checks to prevent kref underflow
@ 2026-06-26 14:54 ` WenTao Liang
0 siblings, 0 replies; 3+ messages in thread
From: WenTao Liang @ 2026-06-26 14:54 UTC (permalink / raw)
To: Christian Koenig, Huang Rui, dri-devel
Cc: Matthew Auld, Matthew Brost, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Karolina Stolarek,
Amaranath Somalapuram, Arunpravin Paneer Selvam, stable,
linux-kernel, WenTao Liang
Replace KUNIT_EXPECT_EQ with KUNIT_ASSERT_EQ in ttm_bo_validate test cases
where subsequent ttm_bo_fini would cause a kref underflow if the preceding
ttm_bo_init_reserved (or ttm_bo_validate) failed. When those functions fail
they already release their internal references, leaving the refcount at 0.
Continuing to ttm_bo_fini without aborting performs an extra kref_put.
Cc: stable@vger.kernel.org
Fixes: 8bd1ff5ddc7b ("drm/ttm/tests: Test simple BO creation and validation")
Signed-off-by: WenTao Liang <vulab@iscas.ac.cn>
---
.../gpu/drm/ttm/tests/ttm_bo_validate_test.c | 22 +++++++++----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
index 2db221f6fc3a..7c4179f6349c 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
@@ -217,7 +217,7 @@ static void ttm_bo_init_reserved_resv(struct kunit *test)
&dummy_ttm_bo_destroy);
dma_resv_unlock(bo->base.resv);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
KUNIT_EXPECT_PTR_EQ(test, bo->base.resv, &resv);
ttm_resource_free(bo, &bo->resource);
@@ -249,7 +249,7 @@ static void ttm_bo_validate_basic(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo, params->bo_type,
fst_placement, PAGE_SIZE, &ctx_init, NULL,
NULL, &dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
snd_place = ttm_place_kunit_init(test, snd_mem, GPU_BUDDY_TOPDOWN_ALLOCATION);
snd_placement = ttm_placement_kunit_init(test, snd_place, 1);
@@ -395,7 +395,7 @@ static void ttm_bo_validate_same_placement(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo, params->bo_type,
placement, PAGE_SIZE, &ctx_init, NULL,
NULL, &dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
err = ttm_bo_validate(bo, placement, &ctx_val);
dma_resv_unlock(bo->base.resv);
@@ -722,7 +722,7 @@ static void ttm_bo_validate_move_fence_not_signaled(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo, bo_type, placement_init,
PAGE_SIZE, &ctx_init, NULL, NULL,
&dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
ttm_mock_manager_init(priv->ttm_dev, fst_mem, MANAGER_SIZE);
ttm_mock_manager_init(priv->ttm_dev, snd_mem, MANAGER_SIZE);
@@ -840,7 +840,7 @@ static void ttm_bo_validate_happy_evict(struct kunit *test)
err = ttm_bo_validate(bo_val, placement, &ctx_val);
ttm_bo_unreserve(bo_val);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
KUNIT_EXPECT_EQ(test, bos[0].resource->mem_type, mem_type_evict);
KUNIT_EXPECT_TRUE(test, bos[0].ttm->page_flags & TTM_TT_FLAG_ZERO_ALLOC);
KUNIT_EXPECT_TRUE(test, bos[0].ttm->page_flags & TTM_TT_FLAG_PRIV_POPULATED);
@@ -879,7 +879,7 @@ static void ttm_bo_validate_all_pinned_evict(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo_big, bo_type, placement,
PAGE_SIZE, &ctx_init, NULL, NULL,
&dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
ttm_bo_pin(bo_big);
dma_resv_unlock(bo_big->base.resv);
@@ -930,7 +930,7 @@ static void ttm_bo_validate_allowed_only_evict(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo_pinned, bo_type, placement,
PAGE_SIZE, &ctx_init, NULL, NULL,
&dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
ttm_bo_pin(bo_pinned);
dma_resv_unlock(bo_pinned->base.resv);
@@ -941,7 +941,7 @@ static void ttm_bo_validate_allowed_only_evict(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo_evictable, bo_type, placement,
PAGE_SIZE, &ctx_init, NULL, NULL,
&dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
dma_resv_unlock(bo_evictable->base.resv);
bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
@@ -995,7 +995,7 @@ static void ttm_bo_validate_deleted_evict(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo_big, bo_type, placement,
PAGE_SIZE, &ctx_init, NULL, NULL,
&dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
KUNIT_EXPECT_EQ(test, ttm_resource_manager_usage(man), big);
dma_resv_unlock(bo_big->base.resv);
@@ -1052,7 +1052,7 @@ static void ttm_bo_validate_busy_domain_evict(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo_init, bo_type, placement,
PAGE_SIZE, &ctx_init, NULL, NULL,
&dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
dma_resv_unlock(bo_init->base.resv);
bo_val = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
@@ -1096,7 +1096,7 @@ static void ttm_bo_validate_evict_gutting(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo_evict, bo_type, placement,
PAGE_SIZE, &ctx_init, NULL, NULL,
&dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
dma_resv_unlock(bo_evict->base.resv);
bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
--
2.39.5 (Apple Git-154)
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH] fix: drm/ttm/tests: use KUNIT_ASSERT_EQ for critical error checks to prevent kref underflow
@ 2026-06-26 14:54 ` WenTao Liang
0 siblings, 0 replies; 3+ messages in thread
From: WenTao Liang @ 2026-06-26 14:54 UTC (permalink / raw)
To: Christian Koenig, Huang Rui, dri-devel
Cc: Matthew Auld, Matthew Brost, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Karolina Stolarek,
Amaranath Somalapuram, Arunpravin Paneer Selvam, stable,
linux-kernel, WenTao Liang
Replace KUNIT_EXPECT_EQ with KUNIT_ASSERT_EQ in ttm_bo_validate test cases
where subsequent ttm_bo_fini would cause a kref underflow if the preceding
ttm_bo_init_reserved (or ttm_bo_validate) failed. When those functions fail
they already release their internal references, leaving the refcount at 0.
Continuing to ttm_bo_fini without aborting performs an extra kref_put.
Cc: stable@vger.kernel.org
Fixes: 8bd1ff5ddc7b ("drm/ttm/tests: Test simple BO creation and validation")
Signed-off-by: WenTao Liang <vulab@iscas.ac.cn>
---
.../gpu/drm/ttm/tests/ttm_bo_validate_test.c | 22 +++++++++----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
index 2db221f6fc3a..7c4179f6349c 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
@@ -217,7 +217,7 @@ static void ttm_bo_init_reserved_resv(struct kunit *test)
&dummy_ttm_bo_destroy);
dma_resv_unlock(bo->base.resv);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
KUNIT_EXPECT_PTR_EQ(test, bo->base.resv, &resv);
ttm_resource_free(bo, &bo->resource);
@@ -249,7 +249,7 @@ static void ttm_bo_validate_basic(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo, params->bo_type,
fst_placement, PAGE_SIZE, &ctx_init, NULL,
NULL, &dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
snd_place = ttm_place_kunit_init(test, snd_mem, GPU_BUDDY_TOPDOWN_ALLOCATION);
snd_placement = ttm_placement_kunit_init(test, snd_place, 1);
@@ -395,7 +395,7 @@ static void ttm_bo_validate_same_placement(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo, params->bo_type,
placement, PAGE_SIZE, &ctx_init, NULL,
NULL, &dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
err = ttm_bo_validate(bo, placement, &ctx_val);
dma_resv_unlock(bo->base.resv);
@@ -722,7 +722,7 @@ static void ttm_bo_validate_move_fence_not_signaled(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo, bo_type, placement_init,
PAGE_SIZE, &ctx_init, NULL, NULL,
&dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
ttm_mock_manager_init(priv->ttm_dev, fst_mem, MANAGER_SIZE);
ttm_mock_manager_init(priv->ttm_dev, snd_mem, MANAGER_SIZE);
@@ -840,7 +840,7 @@ static void ttm_bo_validate_happy_evict(struct kunit *test)
err = ttm_bo_validate(bo_val, placement, &ctx_val);
ttm_bo_unreserve(bo_val);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
KUNIT_EXPECT_EQ(test, bos[0].resource->mem_type, mem_type_evict);
KUNIT_EXPECT_TRUE(test, bos[0].ttm->page_flags & TTM_TT_FLAG_ZERO_ALLOC);
KUNIT_EXPECT_TRUE(test, bos[0].ttm->page_flags & TTM_TT_FLAG_PRIV_POPULATED);
@@ -879,7 +879,7 @@ static void ttm_bo_validate_all_pinned_evict(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo_big, bo_type, placement,
PAGE_SIZE, &ctx_init, NULL, NULL,
&dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
ttm_bo_pin(bo_big);
dma_resv_unlock(bo_big->base.resv);
@@ -930,7 +930,7 @@ static void ttm_bo_validate_allowed_only_evict(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo_pinned, bo_type, placement,
PAGE_SIZE, &ctx_init, NULL, NULL,
&dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
ttm_bo_pin(bo_pinned);
dma_resv_unlock(bo_pinned->base.resv);
@@ -941,7 +941,7 @@ static void ttm_bo_validate_allowed_only_evict(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo_evictable, bo_type, placement,
PAGE_SIZE, &ctx_init, NULL, NULL,
&dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
dma_resv_unlock(bo_evictable->base.resv);
bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
@@ -995,7 +995,7 @@ static void ttm_bo_validate_deleted_evict(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo_big, bo_type, placement,
PAGE_SIZE, &ctx_init, NULL, NULL,
&dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
KUNIT_EXPECT_EQ(test, ttm_resource_manager_usage(man), big);
dma_resv_unlock(bo_big->base.resv);
@@ -1052,7 +1052,7 @@ static void ttm_bo_validate_busy_domain_evict(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo_init, bo_type, placement,
PAGE_SIZE, &ctx_init, NULL, NULL,
&dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
dma_resv_unlock(bo_init->base.resv);
bo_val = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
@@ -1096,7 +1096,7 @@ static void ttm_bo_validate_evict_gutting(struct kunit *test)
err = ttm_bo_init_reserved(priv->ttm_dev, bo_evict, bo_type, placement,
PAGE_SIZE, &ctx_init, NULL, NULL,
&dummy_ttm_bo_destroy);
- KUNIT_EXPECT_EQ(test, err, 0);
+ KUNIT_ASSERT_EQ(test, err, 0);
dma_resv_unlock(bo_evict->base.resv);
bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
--
2.39.5 (Apple Git-154)
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] fix: drm/ttm/tests: use KUNIT_ASSERT_EQ for critical error checks to prevent kref underflow
2026-06-26 14:54 ` WenTao Liang
(?)
@ 2026-06-26 15:06 ` sashiko-bot
-1 siblings, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-06-26 15:06 UTC (permalink / raw)
To: WenTao Liang; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Using KUNIT_ASSERT_EQ aborts the test function on failure and bypasses required manual resource teardown.
- [Medium] Erroneous fix replacing KUNIT_EXPECT_EQ with KUNIT_ASSERT_EQ after ttm_bo_validate() causes a leaked BO reference and potential Use-After-Free.
--
commit af379b6fc27176bd6e80498788a4a528ca8f7559
Author: WenTao Liang <vulab@iscas.ac.cn>
fix: drm/ttm/tests: use KUNIT_ASSERT_EQ for critical error checks to prevent kref underflow
Replace KUNIT_EXPECT_EQ with KUNIT_ASSERT_EQ in ttm_bo_validate test cases
where subsequent ttm_bo_fini would cause a kref underflow if the preceding
ttm_bo_init_reserved (or ttm_bo_validate) failed. When those functions fail
they already release their internal references, leaving the refcount at 0.
Continuing to ttm_bo_fini without aborting performs an extra kref_put.
Cc: stable@vger.kernel.org
Fixes: 8bd1ff5ddc7b ("drm/ttm/tests: Test simple BO creation and validation")
> Replace KUNIT_EXPECT_EQ with KUNIT_ASSERT_EQ in ttm_bo_validate test cases
> where subsequent ttm_bo_fini would cause a kref underflow if the preceding
> ttm_bo_init_reserved (or ttm_bo_validate) failed. When those functions fail
> they already release their internal references, leaving the refcount at 0.
Is this description accurate regarding ttm_bo_validate()?
Looking at ttm_bo_validate(), it does not drop the BO reference on its error
paths. Only ttm_bo_init_reserved() does this, so a failure in
ttm_bo_validate() would leave the reference intact.
> diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> index 2db221f6fc3a1..7c4179f6349c5 100644
> --- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
[ ... ]
> @@ -249,7 +249,7 @@ static void ttm_bo_validate_basic(struct kunit *test)
> err = ttm_bo_init_reserved(priv->ttm_dev, bo, params->bo_type,
> fst_placement, PAGE_SIZE, &ctx_init, NULL,
> NULL, &dummy_ttm_bo_destroy);
> - KUNIT_EXPECT_EQ(test, err, 0);
> + KUNIT_ASSERT_EQ(test, err, 0);
[Severity: Medium]
Will this leak the mock managers on failure?
Because KUNIT_ASSERT_EQ() aborts the test function immediately upon failure,
if ttm_bo_init_reserved() fails here, execution skips the
ttm_mock_manager_fini() cleanup at the end of ttm_bo_validate_basic().
This could leave dangling mock managers in the global state.
This pattern is also present in ttm_bo_validate_same_placement(),
ttm_bo_validate_all_pinned_evict(), and ttm_bo_validate_allowed_only_evict(),
where manual cleanups and mock manager teardowns would also be skipped.
[ ... ]
> @@ -840,7 +840,7 @@ static void ttm_bo_validate_happy_evict(struct kunit *test)
> err = ttm_bo_validate(bo_val, placement, &ctx_val);
> ttm_bo_unreserve(bo_val);
>
> - KUNIT_EXPECT_EQ(test, err, 0);
> + KUNIT_ASSERT_EQ(test, err, 0);
[Severity: Medium]
Does this skip necessary cleanup and leak the buffer object?
As noted above, ttm_bo_validate() does not drop the BO reference on failure.
If it fails here, aborting the test skips the mandatory ttm_bo_fini() loops
at the end of ttm_bo_validate_happy_evict().
While the test array is freed by KUnit on test exit, the leaked elements
might still be referenced by the DRM device's VMA manager. This could
potentially cause a use-after-free when the device is destroyed.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626145409.49370-1-vulab@iscas.ac.cn?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-26 15:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-26 14:54 [PATCH] fix: drm/ttm/tests: use KUNIT_ASSERT_EQ for critical error checks to prevent kref underflow WenTao Liang
2026-06-26 14:54 ` WenTao Liang
2026-06-26 15:06 ` sashiko-bot
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.