[PATCH] linux-user: fix readlinkat handling with magic exe symlink

Jameson Nash posted 1 patch 3 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220808190727.875155-1-vtjnash@gmail.com
Maintainers: Laurent Vivier <laurent@vivier.eu>
linux-user/syscall.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
[PATCH] linux-user: fix readlinkat handling with magic exe symlink
Posted by Jameson Nash 3 years, 6 months ago
Exactly the same as f17f4989fa193fa8279474c5462289a3cfe69aea before was
for readlink. I suppose this was simply missed at the time.

Signed-off-by: Jameson Nash <vtjnash@gmail.com>
---
 linux-user/syscall.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ef53feb5ab..6ef4e42b21 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9894,11 +9894,22 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
             p2 = lock_user(VERIFY_WRITE, arg3, arg4, 0);
             if (!p || !p2) {
                 ret = -TARGET_EFAULT;
+            } else if (!arg4) {
+                /* Short circuit this for the magic exe check. */
+                ret = -TARGET_EINVAL;
             } else if (is_proc_myself((const char *)p, "exe")) {
                 char real[PATH_MAX], *temp;
                 temp = realpath(exec_path, real);
-                ret = temp == NULL ? get_errno(-1) : strlen(real) ;
-                snprintf((char *)p2, arg4, "%s", real);
+                /* Return value is # of bytes that we wrote to the buffer. */
+                if (temp == NULL) {
+                    ret = get_errno(-1);
+                } else {
+                    /* Don't worry about sign mismatch as earlier mapping
+                     * logic would have thrown a bad address error. */
+                    ret = MIN(strlen(real), arg4);
+                    /* We cannot NUL terminate the string. */
+                    memcpy(p2, real, ret);
+                }
             } else {
                 ret = get_errno(readlinkat(arg1, path(p), p2, arg4));
             }
-- 
2.25.1
Re: [PATCH] linux-user: fix readlinkat handling with magic exe symlink
Posted by Laurent Vivier 3 years, 4 months ago
Le 08/08/2022 à 21:07, Jameson Nash a écrit :
> Exactly the same as f17f4989fa193fa8279474c5462289a3cfe69aea before was
> for readlink. I suppose this was simply missed at the time.
> 
> Signed-off-by: Jameson Nash <vtjnash@gmail.com>
> ---
>   linux-user/syscall.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ef53feb5ab..6ef4e42b21 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9894,11 +9894,22 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
>               p2 = lock_user(VERIFY_WRITE, arg3, arg4, 0);
>               if (!p || !p2) {
>                   ret = -TARGET_EFAULT;
> +            } else if (!arg4) {
> +                /* Short circuit this for the magic exe check. */
> +                ret = -TARGET_EINVAL;
>               } else if (is_proc_myself((const char *)p, "exe")) {
>                   char real[PATH_MAX], *temp;
>                   temp = realpath(exec_path, real);
> -                ret = temp == NULL ? get_errno(-1) : strlen(real) ;
> -                snprintf((char *)p2, arg4, "%s", real);
> +                /* Return value is # of bytes that we wrote to the buffer. */
> +                if (temp == NULL) {
> +                    ret = get_errno(-1);
> +                } else {
> +                    /* Don't worry about sign mismatch as earlier mapping
> +                     * logic would have thrown a bad address error. */
> +                    ret = MIN(strlen(real), arg4);
> +                    /* We cannot NUL terminate the string. */
> +                    memcpy(p2, real, ret);
> +                }
>               } else {
>                   ret = get_errno(readlinkat(arg1, path(p), p2, arg4));
>               }

Applied to my linux-user-for-7.2 branch.

Thanks,
Laurent


Re: [PATCH] linux-user: fix readlinkat handling with magic exe symlink
Posted by Laurent Vivier 3 years, 4 months ago
Le 08/08/2022 à 21:07, Jameson Nash a écrit :
> Exactly the same as f17f4989fa193fa8279474c5462289a3cfe69aea before was
> for readlink. I suppose this was simply missed at the time.
> 
> Signed-off-by: Jameson Nash <vtjnash@gmail.com>
> ---
>   linux-user/syscall.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ef53feb5ab..6ef4e42b21 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9894,11 +9894,22 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
>               p2 = lock_user(VERIFY_WRITE, arg3, arg4, 0);
>               if (!p || !p2) {
>                   ret = -TARGET_EFAULT;
> +            } else if (!arg4) {
> +                /* Short circuit this for the magic exe check. */
> +                ret = -TARGET_EINVAL;
>               } else if (is_proc_myself((const char *)p, "exe")) {
>                   char real[PATH_MAX], *temp;
>                   temp = realpath(exec_path, real);
> -                ret = temp == NULL ? get_errno(-1) : strlen(real) ;
> -                snprintf((char *)p2, arg4, "%s", real);
> +                /* Return value is # of bytes that we wrote to the buffer. */
> +                if (temp == NULL) {
> +                    ret = get_errno(-1);
> +                } else {
> +                    /* Don't worry about sign mismatch as earlier mapping
> +                     * logic would have thrown a bad address error. */
> +                    ret = MIN(strlen(real), arg4);
> +                    /* We cannot NUL terminate the string. */
> +                    memcpy(p2, real, ret);
> +                }
>               } else {
>                   ret = get_errno(readlinkat(arg1, path(p), p2, arg4));
>               }

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH] linux-user: fix readlinkat handling with magic exe symlink
Posted by Jameson Nash 3 years, 5 months ago
bump? This helps fix one of the libuv tests when run under qemu
https://github.com/libuv/libuv/pull/2941#issuecomment-1207145306

On Mon, Aug 8, 2022 at 3:07 PM Jameson Nash <vtjnash@gmail.com> wrote:

> Exactly the same as f17f4989fa193fa8279474c5462289a3cfe69aea before was
> for readlink. I suppose this was simply missed at the time.
>
> Signed-off-by: Jameson Nash <vtjnash@gmail.com>
> ---
>  linux-user/syscall.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ef53feb5ab..6ef4e42b21 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9894,11 +9894,22 @@ static abi_long do_syscall1(CPUArchState *cpu_env,
> int num, abi_long arg1,
>              p2 = lock_user(VERIFY_WRITE, arg3, arg4, 0);
>              if (!p || !p2) {
>                  ret = -TARGET_EFAULT;
> +            } else if (!arg4) {
> +                /* Short circuit this for the magic exe check. */
> +                ret = -TARGET_EINVAL;
>              } else if (is_proc_myself((const char *)p, "exe")) {
>                  char real[PATH_MAX], *temp;
>                  temp = realpath(exec_path, real);
> -                ret = temp == NULL ? get_errno(-1) : strlen(real) ;
> -                snprintf((char *)p2, arg4, "%s", real);
> +                /* Return value is # of bytes that we wrote to the
> buffer. */
> +                if (temp == NULL) {
> +                    ret = get_errno(-1);
> +                } else {
> +                    /* Don't worry about sign mismatch as earlier mapping
> +                     * logic would have thrown a bad address error. */
> +                    ret = MIN(strlen(real), arg4);
> +                    /* We cannot NUL terminate the string. */
> +                    memcpy(p2, real, ret);
> +                }
>              } else {
>                  ret = get_errno(readlinkat(arg1, path(p), p2, arg4));
>              }
> --
> 2.25.1
>
>
Re: [PATCH] linux-user: fix readlinkat handling with magic exe symlink
Posted by Jameson Nash 3 years, 4 months ago
bump?? This is just copying the existing
f17f4989fa193fa8279474c5462289a3cfe69aea commit to cover all code paths, so
I was hoping it would be trivial to get an ack for this. Thanks!

On Mon, Aug 29, 2022 at 5:49 PM Jameson Nash <vtjnash@gmail.com> wrote:

> bump? This helps fix one of the libuv tests when run under qemu
> https://github.com/libuv/libuv/pull/2941#issuecomment-1207145306
>
> On Mon, Aug 8, 2022 at 3:07 PM Jameson Nash <vtjnash@gmail.com> wrote:
>
>> Exactly the same as f17f4989fa193fa8279474c5462289a3cfe69aea before was
>> for readlink. I suppose this was simply missed at the time.
>>
>> Signed-off-by: Jameson Nash <vtjnash@gmail.com>
>> ---
>>  linux-user/syscall.c | 15 +++++++++++++--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index ef53feb5ab..6ef4e42b21 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -9894,11 +9894,22 @@ static abi_long do_syscall1(CPUArchState
>> *cpu_env, int num, abi_long arg1,
>>              p2 = lock_user(VERIFY_WRITE, arg3, arg4, 0);
>>              if (!p || !p2) {
>>                  ret = -TARGET_EFAULT;
>> +            } else if (!arg4) {
>> +                /* Short circuit this for the magic exe check. */
>> +                ret = -TARGET_EINVAL;
>>              } else if (is_proc_myself((const char *)p, "exe")) {
>>                  char real[PATH_MAX], *temp;
>>                  temp = realpath(exec_path, real);
>> -                ret = temp == NULL ? get_errno(-1) : strlen(real) ;
>> -                snprintf((char *)p2, arg4, "%s", real);
>> +                /* Return value is # of bytes that we wrote to the
>> buffer. */
>> +                if (temp == NULL) {
>> +                    ret = get_errno(-1);
>> +                } else {
>> +                    /* Don't worry about sign mismatch as earlier mapping
>> +                     * logic would have thrown a bad address error. */
>> +                    ret = MIN(strlen(real), arg4);
>> +                    /* We cannot NUL terminate the string. */
>> +                    memcpy(p2, real, ret);
>> +                }
>>              } else {
>>                  ret = get_errno(readlinkat(arg1, path(p), p2, arg4));
>>              }
>> --
>> 2.25.1
>>
>>