Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CAAo+4rWyNEyoZJ1uAcg7K1ZSHrWiQQO2o_K++uPNMzp-Sy_J7Q@mail.gmail.com>
 <20210801090830.GB18001@1wt.eu> <091e487a-d458-45d4-613d-b25ca7bfc98c@fb.com>
In-Reply-To: <091e487a-d458-45d4-613d-b25ca7bfc98c@fb.com>
From:   =?UTF-8?B?5Y+25r6E6ZSL?= <dg573847474@gmail.com>
Date:   Sat, 28 Aug 2021 16:23:53 +0800
Message-ID: <CAAo+4rX-19UWda_C77goEjq1h10i4-vA=kn-9qy_GgDJVJGq4A@mail.gmail.com>
Subject: Re: Protential deadlock in linux/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
To:     linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Yonghong Song <yhs@fb.com> =E4=BA=8E2021=E5=B9=B48=E6=9C=882=E6=97=A5=E5=91=
=A8=E4=B8=80 =E4=B8=8A=E5=8D=8810:56=E5=86=99=E9=81=93=EF=BC=9A
>
>
>
> 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 deadloc=
k
> >> 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/5bfc75d92efd494db37f5c4c173d363=
9d4772966/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 develope=
r
> > will experience a failure during tests where there should not be. Bette=
r
> > report this directly to the code's authors, without involving the secur=
ity
> > 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 bu=
g?
>
> >
> > Regards,
> > Willy
> >