* [PATCH v2] kunit: fix reference count leak in kfree_at_end
@ 2021-09-09 7:24 Xiyu Yang
2021-09-09 16:59 ` Daniel Latypov
2021-09-28 21:27 ` Brendan Higgins
0 siblings, 2 replies; 5+ messages in thread
From: Xiyu Yang @ 2021-09-09 7:24 UTC (permalink / raw)
To: Brendan Higgins, linux-kselftest, kunit-dev, linux-kernel
Cc: yuanxzhang, Xiyu Yang, Xin Tan
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);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] kunit: fix reference count leak in kfree_at_end
2021-09-09 7:24 [PATCH v2] kunit: fix reference count leak in kfree_at_end Xiyu Yang
@ 2021-09-09 16:59 ` Daniel Latypov
2021-09-09 17:15 ` Daniel Latypov
2021-09-28 21:27 ` Brendan Higgins
1 sibling, 1 reply; 5+ messages in thread
From: Daniel Latypov @ 2021-09-09 16:59 UTC (permalink / raw)
To: Xiyu Yang
Cc: Brendan Higgins, linux-kselftest, kunit-dev, linux-kernel,
yuanxzhang, Xin Tan
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@google.com/.
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-email-xiyuyang19%40fudan.edu.cn.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] kunit: fix reference count leak in kfree_at_end
2021-09-09 16:59 ` Daniel Latypov
@ 2021-09-09 17:15 ` Daniel Latypov
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Latypov @ 2021-09-09 17:15 UTC (permalink / raw)
To: Xiyu Yang
Cc: Brendan Higgins, linux-kselftest, kunit-dev, linux-kernel,
yuanxzhang, Xin Tan
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@google.com/.
> I'll send a v3 fix for that patch as well.
Fixed that patch:
https://lore.kernel.org/linux-kselftest/20210909171052.3192326-2-dlatypov@google.com/
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-email-xiyuyang19%40fudan.edu.cn.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] kunit: fix reference count leak in kfree_at_end
2021-09-09 7:24 [PATCH v2] kunit: fix reference count leak in kfree_at_end Xiyu Yang
2021-09-09 16:59 ` Daniel Latypov
@ 2021-09-28 21:27 ` Brendan Higgins
2021-10-01 19:59 ` Shuah Khan
1 sibling, 1 reply; 5+ messages in thread
From: Brendan Higgins @ 2021-09-28 21:27 UTC (permalink / raw)
To: Xiyu Yang; +Cc: linux-kselftest, kunit-dev, linux-kernel, yuanxzhang, Xin Tan
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!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] kunit: fix reference count leak in kfree_at_end
2021-09-28 21:27 ` Brendan Higgins
@ 2021-10-01 19:59 ` Shuah Khan
0 siblings, 0 replies; 5+ messages in thread
From: Shuah Khan @ 2021-10-01 19:59 UTC (permalink / raw)
To: Xiyu Yang
Cc: linux-kselftest, kunit-dev, linux-kernel, yuanxzhang, Xin Tan,
Brendan Higgins, Shuah Khan
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-01 19:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-09 7:24 [PATCH v2] kunit: fix reference count leak in kfree_at_end Xiyu Yang
2021-09-09 16:59 ` Daniel Latypov
2021-09-09 17:15 ` Daniel Latypov
2021-09-28 21:27 ` Brendan Higgins
2021-10-01 19:59 ` Shuah Khan
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.