Re: [Qemu-devel] [PATCH v5 8/8] linux-user: Split out some process syscalls

[Laurent Vivier <laurent@vivier.eu>]

On 19/12/2018 05:21, Richard Henderson wrote:
> This includes clone, getgroups, gettid, setfsgid, setfsuid,
> setgroups, setsid, setuid, fork, getegid, getegid32, geteuid,
> geteuid32, getgid, getgid32, getgroups32, getpgrp, getpid,
> getppid, getresgid, getresgid32, getresuid, getresuid32,
> getuid, getuid32, getxgid, getxpid, getxuid, setfsgid32,
> setgsuid32, setgid32, setgroups32, setregid, setregid32,
> setresgid, setresgid32, setresuid, setresuid32, setreuid,
> setreuid32, setuid32, vfork.

I have errors with getgroups.

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/syscall-defs.h     | 121 +++++
>  linux-user/syscall.h          |   1 +
>  linux-user/strace.c           |  36 +-
>  linux-user/syscall-proc.inc.c | 861 ++++++++++++++++++++++++++++++++++
>  linux-user/syscall.c          | 677 +-------------------------
>  linux-user/strace.list        | 147 ------
>  6 files changed, 988 insertions(+), 855 deletions(-)
>  create mode 100644 linux-user/syscall-proc.inc.c
...
> diff --git a/linux-user/syscall-proc.inc.c b/linux-user/syscall-proc.inc.c
> new file mode 100644
> index 0000000000..dee441b4ff
> --- /dev/null
> +++ b/linux-user/syscall-proc.inc.c
...
> +
> +SYSCALL_IMPL(getgroups)
> +{
> +    int gidsetsize = arg1;
> +    gid_t *grouplist;
> +    abi_long ret;
> +

kernel checks for gidsetsize < 0 and returns EINVAL in this case

> +    grouplist = g_try_new(gid_t, gidsetsize);
> +    if (!grouplist) {
> +        return -TARGET_ENOMEM;
> +    }

gidsetsize == 0 is a valid case (see man) but it fails with g_try_new().
Moreover, ENOMEM is not a valid error value for getgroups().

> +    ret = get_errno(getgroups(gidsetsize, grouplist));
> +
> +    if (!is_error(ret) && gidsetsize != 0) {
> +        size_t target_grouplist_size = gidsetsize * sizeof(target_id);
> +        target_id *target_grouplist
> +            = lock_user(VERIFY_WRITE, arg2, target_grouplist_size, 0);
> +        if (target_grouplist) {
> +            int i;
> +            for (i = 0; i < ret; i++) {
> +                target_grouplist[i] = tswapid(high2lowgid(grouplist[i]));
> +            }
> +            unlock_user(target_grouplist, arg2, target_grouplist_size);
> +        } else {
> +            ret = -TARGET_EFAULT;
> +        }
> +    }
> +    g_free(grouplist);
> +    return ret;
> +}
> +
> +#ifdef TARGET_NR_getgroups32
> +SYSCALL_IMPL(getgroups32)

likewise.

Thanks,
Laurent