[PATCH bpf v2 09/15] selftests/bpf: Fix double thread join in uprobe_multi_test

Ihor Solodrai posted 15 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH bpf v2 09/15] selftests/bpf: Fix double thread join in uprobe_multi_test
Posted by Ihor Solodrai 1 month, 2 weeks ago
ASAN reported a "joining already joined thread" error. The
release_child() may be called multiple times for the same struct
child.

Fix with a memset(0) call at the end of release_child().

Acked-by: Mykyta Yatsenko <yatsenko@meta.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
index 2ee17ef1dae2..be6ff1d4a75b 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
@@ -68,6 +68,7 @@ static void release_child(struct child *child)
 	close(child->c2p[1]);
 	if (child->pid > 0)
 		waitpid(child->pid, &child_status, 0);
+	memset(child, 0, sizeof(*child));
 }
 
 static void kick_child(struct child *child)
-- 
2.53.0
Re: [PATCH bpf v2 09/15] selftests/bpf: Fix double thread join in uprobe_multi_test
Posted by Ihor Solodrai 1 month, 1 week ago
On 2/17/26 4:30 PM, Ihor Solodrai wrote:
> ASAN reported a "joining already joined thread" error. The
> release_child() may be called multiple times for the same struct
> child.
> 
> Fix with a memset(0) call at the end of release_child().
> 
> Acked-by: Mykyta Yatsenko <yatsenko@meta.com>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> ---
>  tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> index 2ee17ef1dae2..be6ff1d4a75b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> @@ -68,6 +68,7 @@ static void release_child(struct child *child)
>  	close(child->c2p[1]);
>  	if (child->pid > 0)
>  		waitpid(child->pid, &child_status, 0);
> +	memset(child, 0, sizeof(*child));

The CI is failing because of this change.

Apparently, there are asserts on child object after it's released in
kick_child(), so we can't just memset it (without changing the test
logic).  The previous version of the fix [1], resetting only
child->thread, is more appropriate I think.

I missed this because uprobe_multi is in DENYLIST.asan

I'll wait a bit in case there is more feedback and send a v3.

[1] https://lore.kernel.org/bpf/20260212011356.3266753-10-ihor.solodrai@linux.dev/

>  }
>  
>  static void kick_child(struct child *child)
Re: [PATCH bpf v2 09/15] selftests/bpf: Fix double thread join in uprobe_multi_testg
Posted by Jiri Olsa 1 month, 1 week ago
On Wed, Feb 18, 2026 at 09:54:41AM -0800, Ihor Solodrai wrote:
> On 2/17/26 4:30 PM, Ihor Solodrai wrote:
> > ASAN reported a "joining already joined thread" error. The
> > release_child() may be called multiple times for the same struct
> > child.
> > 
> > Fix with a memset(0) call at the end of release_child().
> > 
> > Acked-by: Mykyta Yatsenko <yatsenko@meta.com>
> > Acked-by: Jiri Olsa <jolsa@kernel.org>
> > Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> > ---
> >  tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> > index 2ee17ef1dae2..be6ff1d4a75b 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> > @@ -68,6 +68,7 @@ static void release_child(struct child *child)
> >  	close(child->c2p[1]);
> >  	if (child->pid > 0)
> >  		waitpid(child->pid, &child_status, 0);
> > +	memset(child, 0, sizeof(*child));
> 
> The CI is failing because of this change.

it's on that child->pid,child->tid check in uprobe_multi_test_run?

> 
> Apparently, there are asserts on child object after it's released in
> kick_child(), so we can't just memset it (without changing the test
> logic).  The previous version of the fix [1], resetting only
> child->thread, is more appropriate I think.
> 
> I missed this because uprobe_multi is in DENYLIST.asan
> 
> I'll wait a bit in case there is more feedback and send a v3.

I think we should somehow separate the thread logic from the child to
make the test more clear.. I'll take a look

for now zero-ing the thread, so there's no double join makes sense to me

thanks,
jirka

> 
> [1] https://lore.kernel.org/bpf/20260212011356.3266753-10-ihor.solodrai@linux.dev/
> 
> >  }
> >  
> >  static void kick_child(struct child *child)
> 
>
Re: [PATCH bpf v2 09/15] selftests/bpf: Fix double thread join in uprobe_multi_test
Posted by Mykyta Yatsenko 1 month, 1 week ago
On 2/18/26 17:54, Ihor Solodrai wrote:
> On 2/17/26 4:30 PM, Ihor Solodrai wrote:
>> ASAN reported a "joining already joined thread" error. The
>> release_child() may be called multiple times for the same struct
>> child.
>>
>> Fix with a memset(0) call at the end of release_child().
>>
>> Acked-by: Mykyta Yatsenko <yatsenko@meta.com>
>> Acked-by: Jiri Olsa <jolsa@kernel.org>
>> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
>> ---
>>   tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
>> index 2ee17ef1dae2..be6ff1d4a75b 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
>> @@ -68,6 +68,7 @@ static void release_child(struct child *child)
>>   	close(child->c2p[1]);
>>   	if (child->pid > 0)
>>   		waitpid(child->pid, &child_status, 0);
>> +	memset(child, 0, sizeof(*child));
> The CI is failing because of this change.
>
> Apparently, there are asserts on child object after it's released in
> kick_child(), so we can't just memset it (without changing the test
> logic).  The previous version of the fix [1], resetting only
> child->thread, is more appropriate I think.
>
> I missed this because uprobe_multi is in DENYLIST.asan
>
> I'll wait a bit in case there is more feedback and send a v3.
I think it's more logical to memset it before the test run,
not after (in release_child()).
>
> [1] https://lore.kernel.org/bpf/20260212011356.3266753-10-ihor.solodrai@linux.dev/
>
>>   }
>>   
>>   static void kick_child(struct child *child)
>
>