Re: Protential deadlock in linux/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c

Re: Protential deadlock in linux/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c

[叶澄锋]

Yonghong Song <yhs@fb.com> 于2021年8月2日周一 上午10:56写道：
>
>
>
> On 8/1/21 2:08 AM, Willy Tarreau wrote:
> > Hello,
> >
> > On Sun, Aug 01, 2021 at 04:56:37PM +0800, ??? wrote:
> >> Dear Maintainers,
> >>
> >>
> >> [image: image.png]
> >> This code snippet seems to be problematic since it may cause a deadlock
> >> problem.
> >>
> >> Description:
> >> When *pthread_create* in line 197 fails, *server_started_mtx* will not be
> >> released. If this function is invoked again then, the executing thread will
> >> stuck in line 196 for requiring an already locked mutex, which causes a
> >> deadlock problem.
> >>
> >> Fix suggestions:
> >> Unlock *server_started_mtx *before goto close_server_fd at line 199.
> >>
> >> CWE ID:
> >> CWE-667: Improper Locking
> >>
> >> Source code address:
> >> https://github.com/torvalds/linux/blob/5bfc75d92efd494db37f5c4c173d3639d4772966/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c#L199
> >
> > Sorry, but how is this in any way related to security, given that it's in
> > self-tests programs running in userland ? In the worst case, a developer
> > will experience a failure during tests where there should not be. Better
> > report this directly to the code's authors, without involving the security
> > team. Even better, please provide a patch to fix this.
>
> Yes, this is bpf selftest bug. The fix should be straightforward as
> described in the above.
>
> --- a/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
> @@ -195,8 +195,10 @@ static void run_test(int cgroup_fd)
>
>          pthread_mutex_lock(&server_started_mtx);
>          if (CHECK_FAIL(pthread_create(&tid, NULL, server_thread,
> -                                     (void *)&server_fd)))
> +                                     (void *)&server_fd))) {
> +               pthread_mutex_unlock(&server_started_mtx);
>                  goto close_server_fd;
> +       }
>          pthread_cond_wait(&server_started, &server_started_mtx);
>          pthread_mutex_unlock(&server_started_mtx);
>
> Agree with Willy, maybe you or somebody can provide a patch to fix the bug?
>
> >
> > Regards,
> > Willy
> >