Re: [PATCH] linux-user/syscall: add support for CLONE_PIDFD

[Peter Maydell <peter.maydell@linaro.org>]

On Tue, 21 Sept 2021 at 19:50, Andreas Schwab <schwab@suse.de> wrote:
>
> Add basic support for CLONE_PIDFD, only fork-like clone without additional
> flags.  This is enough to make Qt/forkfd working.
>
> Signed-off-by: Andreas Schwab <schwab@suse.de>
> ---
>  linux-user/syscall.c | 52 ++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 50 insertions(+), 2 deletions(-)

>  /* We can't directly call the host clone syscall, because this will
>   * badly confuse libc (breaking mutexes, for example). So we must

So, this pre-existing comment says "we can't directly call the
host clone syscall"...

> +#if defined __NR_clone2
> +#define __NR_sys_clone2 __NR_clone2
> +_syscall6(int, sys_clone2, int, flags, void *, child_stack, size_t, stack_size,
> +          int *, ptid, int *, ctid, void *, newtls);
> +#else
> +#define __NR_sys_clone __NR_clone
> +#if defined __cris__ || defined __s390x__
> +_syscall5(int, sys_clone, void *, child_stack, int, flags, int *, ptid,
> +          void *, newtls, int *, ctid);
> +#elif defined __microblaze__
> +_syscall6(int, sys_clone, int, flags, void *, child_stack, size_t, stack_size,
> +          int *, ptid, void *, newtls, int *, ctid);
> +#else
> +/*
> + * Note: ctid and newtls are swapped on some architectures, but both are
> + * passed as NULL only for now.
> + */
> +_syscall5(int, sys_clone, int, flags, void *, child_stack, int *, ptid,
> +          int *, ctid, void *, newtls);
> +#endif
> +#endif
> +static int sys_clone_pidfd(int flags, int *pidfd)
> +{
> +#ifdef __NR_clone2
> +    return sys_clone2(flags, NULL, 0, pidfd, NULL, NULL);
> +#elif defined __cris__ || defined __s390x__
> +    return sys_clone(NULL, flags, pidfd, NULL, NULL);
> +#elif defined __microblaze__
> +    return sys_clone(flags, NULL, 0, pidfd, NULL, NULL);
> +#else
> +    return sys_clone(flags, NULL, pidfd, NULL, NULL);
> +#endif
> +}

...but this patch introduces code which directly calls the host
clone syscall.

I think this ought to have a bit more explanation and updating
of the existing comments to explain why this is OK.

thanks
-- PMM