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 <[email protected]> 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/[email protected]?part=2
