Re: [PATCH] linux-user: implement execveat

[Laurent Vivier <laurent@vivier.eu>]

Le 31/10/2022 à 09:40, Drew DeVault a écrit :
> References: https://gitlab.com/qemu-project/qemu/-/issues/1007
> Signed-off-by: Drew DeVault <sir@cmpwn.com>
> ---
>   linux-user/syscall.c | 50 ++++++++++++++++++++++++++++++++++++++------
>   1 file changed, 44 insertions(+), 6 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index f55cdebee5..795f7ce4cd 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -634,6 +634,10 @@ safe_syscall4(pid_t, wait4, pid_t, pid, int *, status, int, options, \
>   safe_syscall5(int, waitid, idtype_t, idtype, id_t, id, siginfo_t *, infop, \
>                 int, options, struct rusage *, rusage)
>   safe_syscall3(int, execve, const char *, filename, char **, argv, char **, envp)
> +#if defined(TARGET_NR_execveat)
> +safe_syscall5(int, execveat, int, dirfd, const char *, filename,
> +        char **, argv, char **, envp, int, flags)
> +#endif
>   #if defined(TARGET_NR_select) || defined(TARGET_NR__newselect) || \
>       defined(TARGET_NR_pselect6) || defined(TARGET_NR_pselect6_time64)
>   safe_syscall6(int, pselect6, int, nfds, fd_set *, readfds, fd_set *, writefds, \
> @@ -8748,19 +8752,45 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
>           ret = get_errno(unlinkat(arg1, p, arg3));
>           unlock_user(p, arg2, 0);
>           return ret;
> +#endif
> +#if defined(TARGET_NR_execveat)
> +    case TARGET_NR_execveat:
>   #endif
>       case TARGET_NR_execve:
>           {
>               char **argp, **envp;
> -            int argc, envc;
> +            int argc, envc, dirfd, flags;
>               abi_ulong gp;
>               abi_ulong guest_argp;
>               abi_ulong guest_envp;
>               abi_ulong addr;
> +            abi_long path;
>               char **q;
>   
>               argc = 0;
> -            guest_argp = arg2;
> +
> +            switch (num) {
> +            case TARGET_NR_execve:
> +                path = arg1;
> +                guest_argp = arg2;
> +                guest_envp = arg3;
> +                dirfd = AT_FDCWD;
> +                flags = 0;
> +                break;
> +#if defined(TARGET_NR_execveat)
> +            case TARGET_NR_execveat:
> +                dirfd = arg1;
> +                path = arg2;
> +                guest_argp = arg3;
> +                guest_envp = arg4;
> +                flags = arg5;
> +                break;
> +#endif
> +            default:
> +                // squelch uninitialized variable warnings
> +                abort();
> +            }
> +
>               for (gp = guest_argp; gp; gp += sizeof(abi_ulong)) {
>                   if (get_user_ual(addr, gp))
>                       return -TARGET_EFAULT;
> @@ -8769,7 +8799,6 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
>                   argc++;
>               }
>               envc = 0;
> -            guest_envp = arg3;
>               for (gp = guest_envp; gp; gp += sizeof(abi_ulong)) {
>                   if (get_user_ual(addr, gp))
>                       return -TARGET_EFAULT;
> @@ -8803,7 +8832,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
>               }
>               *q = NULL;
>   
> -            if (!(p = lock_user_string(arg1)))
> +            if (!(p = lock_user_string(path)))
>                   goto execve_efault;
>               /* Although execve() is not an interruptible syscall it is
>                * a special case where we must use the safe_syscall wrapper:
> @@ -8815,8 +8844,17 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
>                * before the execve completes and makes it the other
>                * program's problem.
>                */
> -            ret = get_errno(safe_execve(p, argp, envp));
> -            unlock_user(p, arg1, 0);
> +            switch (num) {
> +            case TARGET_NR_execve:
> +                ret = get_errno(safe_execve(p, argp, envp));
> +                break;
> +#if defined(TARGET_NR_execveat)
> +            case TARGET_NR_execveat:
> +                ret = get_errno(safe_execveat(dirfd, p, argp, envp, flags));
> +                break;
> +#endif
> +            }
> +            unlock_user(p, path, 0);
>   
>               goto execve_end;
>   


I think it would be clearer to write a common function and to call it from execve and execveat, like 
it's in the kernel:

execve:

   do_execveat(AT_FDCWD, filename, argv, envp, 0);

execeveat:

   do_execveat(fd, filename, argv, envp, flags);

Thanks,
Laurent