Re: [PATCH v2] linux-user/syscall: Implement execve without execveat

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 7/5/23 12:26, Michael Tokarev wrote:
> 05.07.2023 12:00, Pierrick Bouvier wrote:
> ...
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 08162cc966..4945ddd7f2 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -659,6 +659,7 @@ safe_syscall4(pid_t, wait4, pid_t, pid, int *, status, int, options, \
>>    #endif
>>    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)
>>    safe_syscall5(int, execveat, int, dirfd, const char *, filename,
>>                  char **, argv, char **, envp, int, flags)
>>    #if defined(TARGET_NR_select) || defined(TARGET_NR__newselect) || \
>> @@ -8615,9 +8616,9 @@ ssize_t do_guest_readlink(const char *pathname, char *buf, size_t bufsiz)
>>        return ret;
>>    }
>>    
>> -static int do_execveat(CPUArchState *cpu_env, int dirfd,
>> -                       abi_long pathname, abi_long guest_argp,
>> -                       abi_long guest_envp, int flags)
>> +static int do_execv(CPUArchState *cpu_env, int dirfd,
>> +                    abi_long pathname, abi_long guest_argp,
>> +                    abi_long guest_envp, int flags, bool is_execveat)
>>    {
>>        int ret;
>>        char **argp, **envp;
>> @@ -8696,11 +8697,14 @@ static int do_execveat(CPUArchState *cpu_env, int dirfd,
>>            goto execve_efault;
>>        }
>>    
>> +    const char* exe = p;
>>        if (is_proc_myself(p, "exe")) {
>> -        ret = get_errno(safe_execveat(dirfd, exec_path, argp, envp, flags));
>> -    } else {
>> -        ret = get_errno(safe_execveat(dirfd, p, argp, envp, flags));
>> +        exe = exec_path;
>>        }
>> +    ret = is_execveat ?
>> +        safe_execveat(dirfd, exe, argp, envp, flags):
>> +        safe_execve(exe, argp, envp);
>> +    ret = get_errno(ret);
> 
> 
> This one has 2 issues reported by checkpatch.pl:
> 
> $ git show | ./scripts/checkpatch.pl -
> 
> ERROR: "foo* bar" should be "foo *bar"
> #161: FILE: linux-user/syscall.c:8700:
> +    const char* exe = p;
> 
> ERROR: spaces required around that ':' (ctx:VxE)
> #169: FILE: linux-user/syscall.c:8705:
> +        safe_execveat(dirfd, exe, argp, envp, flags):
>                                                        ^
> 
> total: 2 errors, 0 warnings, 47 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> As I mentioned in the v1, I don't remember offhand how the arithmetic if
> should be styled in qemu.  At the very best, the v2 variant is difficult
> to read because ":" is too close to ";" visually, an extra space before
> it will make it more explicit.
> 

Sorry about it.
I'll fix it, and use style advised by Philippe.

> Other than that, I'm fine with this version.
> 
> With the checkpatch issues fixed,
> 
> Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
> 
> Thanks,
> 
> /mjt