Re: [PATCH] fix create thread failed in unprivileged process [BZ #28287]

["H.J. Lu" <hjl.tools@gmail.com>]

On Sun, Aug 29, 2021 at 6:29 AM Hongxu Jia <hongxu.jia@windriver.com> wrote:
>
> Since commit [d8ea0d0168 Add an internal wrapper for clone, clone2 and clone3]
> applied, start a unprivileged container (docker run without --privileged),
> it creates a thread failed in container.
>
> In commit d8ea0d0168, it calls __clone3 if HAVE_CLONE3_WAPPER is defined.  If
> __clone3 returns -1 with ENOSYS, fall back to clone or clone2.
>
> As known from [1], cloneXXX fails with EPERM if CLONE_NEWCGROUP,
> CLONE_NEWIPC, CLONE_NEWNET, CLONE_NEWNS, CLONE_NEWPID, or CLONE_NEWUTS
> was specified by an unprivileged process (process without CAP_SYS_ADMIN)

I don't think the description is accurate.  In your test, none
of the mentioned flags are used directly.  The real bug is
that the container you used blocks the normal clone3 and
sets errno to EPERM.  The question is if/how glibc should
work arounds the clone3 bug in containers.   We want to add
a public clone3 wrapper to glibc in the future.  But before we
do that, all these containers should be changed to ENOSYS
if clone3 is blocked.

> [1] https://man7.org/linux/man-pages/man2/clone3.2.html
>
> So if __clone3 returns -1 with EPERM, fall back to clone or clone2 could
> fix the issue. Here are the test steps:
>
> 1) Prepare test code
> cat > conftest.c <<ENDOF
>  #include <pthread.h>
>  #include <stdio.h>
>
> int check_me = 0;
> void* func(void* data) {check_me = 42; printf("start thread: check_me %d\n", check_me); return &check_me;}
> int main()
> {
>   pthread_t t;
>   void *ret;
>   pthread_create (&t, 0, func, 0);
>   pthread_join (t, &ret);
>   printf("check_me %d, p %p\n", check_me, &ret);
>   return (check_me != 42 || ret != &check_me);
> }
>
> ENDOF
>
> 2) Compile
> gcc -o conftest -pthread conftest.c
>
> 3) Start a container with glibc 2.34 installed
> [skip details]
> docker run -it <container-image-name> bash
>
> 4) Run conftest without this patch
> $ ./conftest
> check_me 0, p 0x7ffd91ccd400
>
> 5) Run conftest with this patch
> $ ./conftest
> start thread: check_me 42
> check_me 42, p 0x7ffe253c6f20
>
> Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
> ---
>  sysdeps/unix/sysv/linux/clone-internal.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sysdeps/unix/sysv/linux/clone-internal.c b/sysdeps/unix/sysv/linux/clone-internal.c
> index 979f7880be..97101994e8 100644
> --- a/sysdeps/unix/sysv/linux/clone-internal.c
> +++ b/sysdeps/unix/sysv/linux/clone-internal.c
> @@ -52,7 +52,7 @@ __clone_internal (struct clone_args *cl_args,
>    /* Try clone3 first.  */
>    int saved_errno = errno;
>    ret = __clone3 (cl_args, sizeof (*cl_args), func, arg);
> -  if (ret != -1 || errno != ENOSYS)
> +  if (ret != -1 || (errno != ENOSYS && errno != EPERM))
>      return ret;
>
>    /* NB: Restore errno since errno may be checked against non-zero
> --
> 2.30.2
>


-- 
H.J.