Re: [PATCH 02/10] dma-buf/tests: Convert st-dma-resv tests to use dma_resv_alloc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Memory leak of manually allocated 'dma_fence' object on test failure path due to KUNIT_ASSERT_NOT_NULL aborting the test.
--

commit 2140307021746c828365fffb6d1a6a49b2fe2aa6
Author: Christian König <ckoenig.leichtzumerken@gmail.com>
dma-buf/tests: Convert st-dma-resv tests to use dma_resv_alloc

This patch updates the test cases in st-dma-resv.c to use the new
dma_resv_alloc API instead of stack-allocated objects. It introduces
pointer allocations and adds reference counting verification by calling
dma_resv_put for cleanup.

> diff --git a/drivers/dma-buf/st-dma-resv.c b/drivers/dma-buf/st-dma-resv.c
> index 2c43d2d2b0169..b843d90417822 100644
> --- a/drivers/dma-buf/st-dma-resv.c
> +++ b/drivers/dma-buf/st-dma-resv.c
[ ... ]
> @@ -75,33 +77,35 @@ static void test_signaling(struct kunit *test)
>  
>  	dma_fence_enable_signaling(f);
>  
> -	dma_resv_init(&resv);
> -	r = dma_resv_lock(&resv, NULL);
> +	resv = dma_resv_alloc();
> +	KUNIT_ASSERT_NOT_NULL(test, resv);

[Severity: Medium]
Does this code leak the allocated dma_fence 'f' if the assertion fails?

In test_signaling(), 'f' is allocated right before this code block. If
dma_resv_alloc() fails due to memory pressure, KUNIT_ASSERT_NOT_NULL() will
abort the test execution immediately. This bypasses the err_free cleanup
label and leaves 'f' stranded without its corresponding dma_fence_put() call.

Since this pattern is repeated in the other tests below, would it be
possible to either reorder the allocations or use KUNIT_EXPECT_NOT_NULL()
to allow for proper cleanup on failure?

> +
> +	r = dma_resv_lock(resv, NULL);
[ ... ]
> @@ -119,23 +123,25 @@ static void test_for_each(struct kunit *test)
>  
>  	dma_fence_enable_signaling(f);
>  
> -	dma_resv_init(&resv);
> -	r = dma_resv_lock(&resv, NULL);
> +	resv = dma_resv_alloc();
> +	KUNIT_ASSERT_NOT_NULL(test, resv);

[Severity: Medium]
Similarly here in test_for_each(), does a failure on this assertion
leak the 'f' object?

> +
> +	r = dma_resv_lock(resv, NULL);
[ ... ]
> @@ -178,25 +184,27 @@ static void test_for_each_unlocked(struct kunit *test)
>  
>  	dma_fence_enable_signaling(f);
>  
> -	dma_resv_init(&resv);
> -	r = dma_resv_lock(&resv, NULL);
> +	resv = dma_resv_alloc();
> +	KUNIT_ASSERT_NOT_NULL(test, resv);

[Severity: Medium]
Does this assertion also leak 'f' in test_for_each_unlocked() if it fails?

> +
> +	r = dma_resv_lock(resv, NULL);
[ ... ]
> @@ -248,24 +256,26 @@ static void test_get_fences(struct kunit *test)
>  
>  	dma_fence_enable_signaling(f);
>  
> -	dma_resv_init(&resv);
> -	r = dma_resv_lock(&resv, NULL);
> +	resv = dma_resv_alloc();
> +	KUNIT_ASSERT_NOT_NULL(test, resv);

[Severity: Medium]
Does this assertion also leak 'f' in test_get_fences() if it fails?

> +
> +	r = dma_resv_lock(resv, NULL);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827124910.2245-1-christian.koenig@amd.com?part=2