The reference counting issue happens in the normal path of kfree_at_end(). When kunit_alloc_and_get_resource() is invoked, the function forgets to handle the returned resource object, whose refcount increased inside, causing a refcount leak.
Fix this issue by calling kunit_alloc_resource() instead of kunit_alloc_and_get_resource().
Signed-off-by: Xiyu Yang xiyuyang19@fudan.edu.cn Signed-off-by: Xin Tan tanxin.ctf@gmail.com --- lib/kunit/executor_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c index cdbe54b16501..c2dcfb1f6e97 100644 --- a/lib/kunit/executor_test.c +++ b/lib/kunit/executor_test.c @@ -116,7 +116,7 @@ static void kfree_at_end(struct kunit *test, const void *to_free) /* kfree() handles NULL already, but avoid allocating a no-op cleanup. */ if (IS_ERR_OR_NULL(to_free)) return; - kunit_alloc_and_get_resource(test, NULL, kfree_res_free, GFP_KERNEL, + kunit_alloc_resource(test, NULL, kfree_res_free, GFP_KERNEL, (void *)to_free); }
On Thu, Sep 9, 2021 at 12:26 AM 'Xiyu Yang' via KUnit Development kunit-dev@googlegroups.com wrote:
The reference counting issue happens in the normal path of kfree_at_end(). When kunit_alloc_and_get_resource() is invoked, the function forgets to handle the returned resource object, whose refcount increased inside, causing a refcount leak.
Fix this issue by calling kunit_alloc_resource() instead of kunit_alloc_and_get_resource().
Signed-off-by: Xiyu Yang xiyuyang19@fudan.edu.cn Signed-off-by: Xin Tan tanxin.ctf@gmail.com
Reviewed-by: Daniel Latypov dlatypov@google.com
Ah, thanks for finding and fixing this! We really should have better documentation/otherwise make it clearer that people shouldn't use the "_and_get" version.
I went and added some pr_info() calls to verify that these were being leaked before and they're fixed now.
I copy-pasted this mistake into https://lore.kernel.org/linux-kselftest/20210831171926.3832806-2-dlatypov@go.... I'll send a v3 fix for that patch as well.
lib/kunit/executor_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c index cdbe54b16501..c2dcfb1f6e97 100644 --- a/lib/kunit/executor_test.c +++ b/lib/kunit/executor_test.c @@ -116,7 +116,7 @@ static void kfree_at_end(struct kunit *test, const void *to_free) /* kfree() handles NULL already, but avoid allocating a no-op cleanup. */ if (IS_ERR_OR_NULL(to_free)) return;
kunit_alloc_and_get_resource(test, NULL, kfree_res_free, GFP_KERNEL,
kunit_alloc_resource(test, NULL, kfree_res_free, GFP_KERNEL, (void *)to_free);}
-- 2.7.4
-- You received this message because you are subscribed to the Google Groups "KUnit Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/1631172276-82914-1-git-send-emai....
On Thu, Sep 9, 2021 at 9:59 AM Daniel Latypov dlatypov@google.com wrote:
On Thu, Sep 9, 2021 at 12:26 AM 'Xiyu Yang' via KUnit Development kunit-dev@googlegroups.com wrote:
The reference counting issue happens in the normal path of kfree_at_end(). When kunit_alloc_and_get_resource() is invoked, the function forgets to handle the returned resource object, whose refcount increased inside, causing a refcount leak.
Fix this issue by calling kunit_alloc_resource() instead of kunit_alloc_and_get_resource().
Signed-off-by: Xiyu Yang xiyuyang19@fudan.edu.cn Signed-off-by: Xin Tan tanxin.ctf@gmail.com
Reviewed-by: Daniel Latypov dlatypov@google.com
Ah, thanks for finding and fixing this! We really should have better documentation/otherwise make it clearer that people shouldn't use the "_and_get" version.
I went and added some pr_info() calls to verify that these were being leaked before and they're fixed now.
I copy-pasted this mistake into https://lore.kernel.org/linux-kselftest/20210831171926.3832806-2-dlatypov@go.... I'll send a v3 fix for that patch as well.
Fixed that patch: https://lore.kernel.org/linux-kselftest/20210909171052.3192326-2-dlatypov@go...
I assume this patch and that one shouldn't have merge conflicts, so we don't need these to be applied in any specific order.
lib/kunit/executor_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c index cdbe54b16501..c2dcfb1f6e97 100644 --- a/lib/kunit/executor_test.c +++ b/lib/kunit/executor_test.c @@ -116,7 +116,7 @@ static void kfree_at_end(struct kunit *test, const void *to_free) /* kfree() handles NULL already, but avoid allocating a no-op cleanup. */ if (IS_ERR_OR_NULL(to_free)) return;
kunit_alloc_and_get_resource(test, NULL, kfree_res_free, GFP_KERNEL,
kunit_alloc_resource(test, NULL, kfree_res_free, GFP_KERNEL, (void *)to_free);}
-- 2.7.4
-- You received this message because you are subscribed to the Google Groups "KUnit Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/1631172276-82914-1-git-send-emai....
On Thu, Sep 9, 2021 at 12:26 AM Xiyu Yang xiyuyang19@fudan.edu.cn wrote:
The reference counting issue happens in the normal path of kfree_at_end(). When kunit_alloc_and_get_resource() is invoked, the function forgets to handle the returned resource object, whose refcount increased inside, causing a refcount leak.
Fix this issue by calling kunit_alloc_resource() instead of kunit_alloc_and_get_resource().
Signed-off-by: Xiyu Yang xiyuyang19@fudan.edu.cn Signed-off-by: Xin Tan tanxin.ctf@gmail.com
Reviewed-by: Brendan Higgins brendanhiggins@google.com
Thanks!
On 9/28/21 3:27 PM, Brendan Higgins wrote:
On Thu, Sep 9, 2021 at 12:26 AM Xiyu Yang xiyuyang19@fudan.edu.cn wrote:
The reference counting issue happens in the normal path of kfree_at_end(). When kunit_alloc_and_get_resource() is invoked, the function forgets to handle the returned resource object, whose refcount increased inside, causing a refcount leak.
Fix this issue by calling kunit_alloc_resource() instead of kunit_alloc_and_get_resource().
Signed-off-by: Xiyu Yang xiyuyang19@fudan.edu.cn Signed-off-by: Xin Tan tanxin.ctf@gmail.com
Reviewed-by: Brendan Higgins brendanhiggins@google.com
Thanks!
Thank you for the fix.
Applied now after fixing a checkpatch alignment check to
git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/ kunit-fixes branch.
Please remember to run checkpatch.pl before sending patches.
thanks, -- Shuah
linux-kselftest-mirror@lists.linaro.org
-
Brendan Higgins -
Daniel Latypov -
Shuah Khan -
Xiyu Yang