[PATCH 0/2] linux-user: Fix handling when AT_EXECFD is 0

[PATCH 0/2] linux-user: Fix handling when AT_EXECFD is 0

[Vivian Wang]

Found when trying to build coreutils under linux-user as binfmt_misc
interpreter with "open-binary" flag.

I'm unsure whether the version detection in patch 1 makes sense. Please
advise on how to handle situations like this.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2448

Vivian Wang (2):
  util/getauxval: Ensure setting errno if not found
  linux-user/main: Check errno when getting AT_EXECFD

 linux-user/main.c |  3 ++-
 util/getauxval.c  | 14 ++++++++++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

--
2.45.1

[PATCH 1/2] util/getauxval: Ensure setting errno if not found

[Vivian Wang]

Sometimes zero is a valid value for getauxval (e.g. AT_EXECFD). Make
sure that we can distinguish between a valid zero value and a not found
entry by setting errno.

Ignore getauxval from sys/auxv.h on glibc < 2.19 because it does not set
errno.

Signed-off-by: Vivian Wang <uwu@dram.page>
---
 util/getauxval.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/util/getauxval.c b/util/getauxval.c
index b124107d61..f1008bdc59 100644
--- a/util/getauxval.c
+++ b/util/getauxval.c
@@ -24,7 +24,13 @@
 
 #include "qemu/osdep.h"
 
-#ifdef CONFIG_GETAUXVAL
+/* If glibc < 2.19, getauxval can't be used because it does not set errno if
+   entry is not found. */
+#if defined(CONFIG_GETAUXVAL) && \
+    (!defined(__GLIBC__) \
+        || __GLIBC__ > 2 \
+        || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 19))
+
 /* Don't inline this in qemu/osdep.h, because pulling in <sys/auxv.h> for
    the system declaration of getauxval pulls in the system <elf.h>, which
    conflicts with qemu's version.  */
@@ -95,6 +101,7 @@ unsigned long qemu_getauxval(unsigned long type)
         }
     }
 
+    errno = ENOENT;
     return 0;
 }
 
@@ -104,7 +111,9 @@ unsigned long qemu_getauxval(unsigned long type)
 unsigned long qemu_getauxval(unsigned long type)
 {
     unsigned long aux = 0;
-    elf_aux_info(type, &aux, sizeof(aux));
+    int ret = elf_aux_info(type, &aux, sizeof(aux));
+    if (ret != 0)
+        errno = ret;
     return aux;
 }
 
@@ -112,6 +121,7 @@ unsigned long qemu_getauxval(unsigned long type)
 
 unsigned long qemu_getauxval(unsigned long type)
 {
+    errno = ENOSYS;
     return 0;
 }
 
-- 
2.45.1

[PATCH 2/2] linux-user/main: Check errno when getting AT_EXECFD

[Vivian Wang]

It's possible for AT_EXECFD to end up with a valid value of 0. Check
errno when using qemu_getauxval instead of return value to handle this
case.

Not handling this case leads to a confusing condition where the
executable ends up as fd 0, i.e. stdin.

Signed-off-by: Vivian Wang <uwu@dram.page>
Fixes: 0b959cf5e4cc ("linux-user: Use qemu_getauxval for AT_EXECFD")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2448
---
 linux-user/main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 7d3cf45fa9..8143a0d4b0 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -755,8 +755,9 @@ int main(int argc, char **argv, char **envp)
     /*
      * Manage binfmt-misc open-binary flag
      */
+    errno = 0;
     execfd = qemu_getauxval(AT_EXECFD);
-    if (execfd == 0) {
+    if (errno != 0) {
         execfd = open(exec_path, O_RDONLY);
         if (execfd < 0) {
             printf("Error while loading %s: %s\n", exec_path, strerror(errno));
-- 
2.45.1

RE: [PATCH 1/2] util/getauxval: Ensure setting errno if not found

[Xingtao Yao (Fujitsu) via]

> -----Original Message-----
> From: qemu-devel-bounces+yaoxt.fnst=fujitsu.com@nongnu.org
> <qemu-devel-bounces+yaoxt.fnst=fujitsu.com@nongnu.org> On Behalf Of Vivian
> Wang
> Sent: Sunday, July 21, 2024 5:08 PM
> To: qemu-devel@nongnu.org
> Cc: Vivian Wang <uwu@dram.page>; Richard Henderson <rth@twiddle.net>;
> Laurent Vivier <laurent@vivier.eu>
> Subject: [PATCH 1/2] util/getauxval: Ensure setting errno if not found
> 
> Sometimes zero is a valid value for getauxval (e.g. AT_EXECFD). Make
> sure that we can distinguish between a valid zero value and a not found
> entry by setting errno.
> 
> Ignore getauxval from sys/auxv.h on glibc < 2.19 because it does not set
> errno.
> 
> Signed-off-by: Vivian Wang <uwu@dram.page>
> ---
>  util/getauxval.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/util/getauxval.c b/util/getauxval.c
> index b124107d61..f1008bdc59 100644
> --- a/util/getauxval.c
> +++ b/util/getauxval.c
> @@ -24,7 +24,13 @@
> 
>  #include "qemu/osdep.h"
> 
> -#ifdef CONFIG_GETAUXVAL
> +/* If glibc < 2.19, getauxval can't be used because it does not set errno if
> +   entry is not found. */
> +#if defined(CONFIG_GETAUXVAL) && \
> +    (!defined(__GLIBC__) \
> +        || __GLIBC__ > 2 \
> +        || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 19))
you can use GLIB_CHECK_VERSION(2, 19, 0) instead

> +
>  /* Don't inline this in qemu/osdep.h, because pulling in <sys/auxv.h> for
>     the system declaration of getauxval pulls in the system <elf.h>, which
>     conflicts with qemu's version.  */
> @@ -95,6 +101,7 @@ unsigned long qemu_getauxval(unsigned long type)
>          }
>      }
> 
> +    errno = ENOENT;
>      return 0;
>  }
> 
> @@ -104,7 +111,9 @@ unsigned long qemu_getauxval(unsigned long type)
>  unsigned long qemu_getauxval(unsigned long type)
>  {
>      unsigned long aux = 0;
> -    elf_aux_info(type, &aux, sizeof(aux));
> +    int ret = elf_aux_info(type, &aux, sizeof(aux));
> +    if (ret != 0)
> +        errno = ret;
>      return aux;
>  }
> 
> @@ -112,6 +121,7 @@ unsigned long qemu_getauxval(unsigned long type)
> 
>  unsigned long qemu_getauxval(unsigned long type)
>  {
> +    errno = ENOSYS;
>      return 0;
>  }
> 
> --
> 2.45.1
> 

Re: [PATCH 1/2] util/getauxval: Ensure setting errno if not found

[Vivian Wang]

On 7/22/24 08:18, Xingtao Yao (Fujitsu) wrote:
>
>> -----Original Message-----
>> From: qemu-devel-bounces+yaoxt.fnst=fujitsu.com@nongnu.org
>> <qemu-devel-bounces+yaoxt.fnst=fujitsu.com@nongnu.org> On Behalf Of Vivian
>> Wang
>> Sent: Sunday, July 21, 2024 5:08 PM
>> To: qemu-devel@nongnu.org
>> Cc: Vivian Wang <uwu@dram.page>; Richard Henderson <rth@twiddle.net>;
>> Laurent Vivier <laurent@vivier.eu>
>> Subject: [PATCH 1/2] util/getauxval: Ensure setting errno if not found
>>
>> Sometimes zero is a valid value for getauxval (e.g. AT_EXECFD). Make
>> sure that we can distinguish between a valid zero value and a not found
>> entry by setting errno.
>>
>> Ignore getauxval from sys/auxv.h on glibc < 2.19 because it does not set
>> errno.
>>
>> Signed-off-by: Vivian Wang <uwu@dram.page>
>> ---
>>  util/getauxval.c | 14 ++++++++++++--
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/util/getauxval.c b/util/getauxval.c
>> index b124107d61..f1008bdc59 100644
>> --- a/util/getauxval.c
>> +++ b/util/getauxval.c
>> @@ -24,7 +24,13 @@
>>
>>  #include "qemu/osdep.h"
>>
>> -#ifdef CONFIG_GETAUXVAL
>> +/* If glibc < 2.19, getauxval can't be used because it does not set errno if
>> +   entry is not found. */
>> +#if defined(CONFIG_GETAUXVAL) && \
>> +    (!defined(__GLIBC__) \
>> +        || __GLIBC__ > 2 \
>> +        || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 19))
> you can use GLIB_CHECK_VERSION(2, 19, 0) instead
That wouldn't work. I'm testing for glibc, not glib.
>> +
>>  /* Don't inline this in qemu/osdep.h, because pulling in <sys/auxv.h> for
>>     the system declaration of getauxval pulls in the system <elf.h>, which
>>     conflicts with qemu's version.  */
>> @@ -95,6 +101,7 @@ unsigned long qemu_getauxval(unsigned long type)
>>          }
>>      }
>>
>> +    errno = ENOENT;
>>      return 0;
>>  }
>>
>> @@ -104,7 +111,9 @@ unsigned long qemu_getauxval(unsigned long type)
>>  unsigned long qemu_getauxval(unsigned long type)
>>  {
>>      unsigned long aux = 0;
>> -    elf_aux_info(type, &aux, sizeof(aux));
>> +    int ret = elf_aux_info(type, &aux, sizeof(aux));
>> +    if (ret != 0)
>> +        errno = ret;
>>      return aux;
>>  }
>>
>> @@ -112,6 +121,7 @@ unsigned long qemu_getauxval(unsigned long type)
>>
>>  unsigned long qemu_getauxval(unsigned long type)
>>  {
>> +    errno = ENOSYS;
>>      return 0;
>>  }
>>
>> --
>> 2.45.1
>>

RE: [PATCH 1/2] util/getauxval: Ensure setting errno if not found

[Xingtao Yao (Fujitsu) via]

> -----Original Message-----
> From: Vivian Wang <uwu@dram.page>
> Sent: Monday, July 22, 2024 4:24 PM
> To: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com>; qemu-devel@nongnu.org
> Cc: Richard Henderson <rth@twiddle.net>; Laurent Vivier <laurent@vivier.eu>
> Subject: Re: [PATCH 1/2] util/getauxval: Ensure setting errno if not found
> 
> On 7/22/24 08:18, Xingtao Yao (Fujitsu) wrote:
> >
> >> -----Original Message-----
> >> From: qemu-devel-bounces+yaoxt.fnst=fujitsu.com@nongnu.org
> >> <qemu-devel-bounces+yaoxt.fnst=fujitsu.com@nongnu.org> On Behalf Of
> Vivian
> >> Wang
> >> Sent: Sunday, July 21, 2024 5:08 PM
> >> To: qemu-devel@nongnu.org
> >> Cc: Vivian Wang <uwu@dram.page>; Richard Henderson <rth@twiddle.net>;
> >> Laurent Vivier <laurent@vivier.eu>
> >> Subject: [PATCH 1/2] util/getauxval: Ensure setting errno if not found
> >>
> >> Sometimes zero is a valid value for getauxval (e.g. AT_EXECFD). Make
> >> sure that we can distinguish between a valid zero value and a not found
> >> entry by setting errno.
> >>
> >> Ignore getauxval from sys/auxv.h on glibc < 2.19 because it does not set
> >> errno.
> >>
> >> Signed-off-by: Vivian Wang <uwu@dram.page>
> >> ---
> >>  util/getauxval.c | 14 ++++++++++++--
> >>  1 file changed, 12 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/util/getauxval.c b/util/getauxval.c
> >> index b124107d61..f1008bdc59 100644
> >> --- a/util/getauxval.c
> >> +++ b/util/getauxval.c
> >> @@ -24,7 +24,13 @@
> >>
> >>  #include "qemu/osdep.h"
> >>
> >> -#ifdef CONFIG_GETAUXVAL
> >> +/* If glibc < 2.19, getauxval can't be used because it does not set errno if
> >> +   entry is not found. */
> >> +#if defined(CONFIG_GETAUXVAL) && \
> >> +    (!defined(__GLIBC__) \
> >> +        || __GLIBC__ > 2 \
> >> +        || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 19))
> > you can use GLIB_CHECK_VERSION(2, 19, 0) instead
> That wouldn't work. I'm testing for glibc, not glib.
sorry, I misunderstood.

> >> +
> >>  /* Don't inline this in qemu/osdep.h, because pulling in <sys/auxv.h> for
> >>     the system declaration of getauxval pulls in the system <elf.h>, which
> >>     conflicts with qemu's version.  */
> >> @@ -95,6 +101,7 @@ unsigned long qemu_getauxval(unsigned long type)
> >>          }
> >>      }
> >>
> >> +    errno = ENOENT;
> >>      return 0;
> >>  }
> >>
> >> @@ -104,7 +111,9 @@ unsigned long qemu_getauxval(unsigned long type)
> >>  unsigned long qemu_getauxval(unsigned long type)
> >>  {
> >>      unsigned long aux = 0;
> >> -    elf_aux_info(type, &aux, sizeof(aux));
> >> +    int ret = elf_aux_info(type, &aux, sizeof(aux));
> >> +    if (ret != 0)
> >> +        errno = ret;
> >>      return aux;
> >>  }
> >>
> >> @@ -112,6 +121,7 @@ unsigned long qemu_getauxval(unsigned long type)
> >>
> >>  unsigned long qemu_getauxval(unsigned long type)
> >>  {
> >> +    errno = ENOSYS;
> >>      return 0;
> >>  }
> >>
> >> --
> >> 2.45.1
> >>

Re: [PATCH 1/2] util/getauxval: Ensure setting errno if not found

[Richard Henderson]

On 7/21/24 19:08, Vivian Wang wrote:
> Sometimes zero is a valid value for getauxval (e.g. AT_EXECFD). Make
> sure that we can distinguish between a valid zero value and a not found
> entry by setting errno.
> 
> Ignore getauxval from sys/auxv.h on glibc < 2.19 because it does not set
> errno.
> 
> Signed-off-by: Vivian Wang <uwu@dram.page>
> ---
>   util/getauxval.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/util/getauxval.c b/util/getauxval.c
> index b124107d61..f1008bdc59 100644
> --- a/util/getauxval.c
> +++ b/util/getauxval.c
> @@ -24,7 +24,13 @@
>   
>   #include "qemu/osdep.h"
>   
> -#ifdef CONFIG_GETAUXVAL
> +/* If glibc < 2.19, getauxval can't be used because it does not set errno if
> +   entry is not found. */
> +#if defined(CONFIG_GETAUXVAL) && \
> +    (!defined(__GLIBC__) \
> +        || __GLIBC__ > 2 \
> +        || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 19))

Version 2.19 is so long ago that all supported OS releases have something newer.  For 
Alpine Linux using musl, the first commit of getauxval had the errno setting.

Therefore we don't need this check.

> @@ -95,6 +101,7 @@ unsigned long qemu_getauxval(unsigned long type)
>           }
>       }
>   
> +    errno = ENOENT;
>       return 0;
>   }

Ok.

> @@ -104,7 +111,9 @@ unsigned long qemu_getauxval(unsigned long type)
>   unsigned long qemu_getauxval(unsigned long type)
>   {
>       unsigned long aux = 0;
> -    elf_aux_info(type, &aux, sizeof(aux));
> +    int ret = elf_aux_info(type, &aux, sizeof(aux));
> +    if (ret != 0)
> +        errno = ret;

Braces required.

>       return aux;
>   }
>   
> @@ -112,6 +121,7 @@ unsigned long qemu_getauxval(unsigned long type)
>   
>   unsigned long qemu_getauxval(unsigned long type)
>   {
> +    errno = ENOSYS;
>       return 0;
>   }

Ok.


r~

Re: [PATCH 2/2] linux-user/main: Check errno when getting AT_EXECFD

[Richard Henderson]

On 7/21/24 19:08, Vivian Wang wrote:
> It's possible for AT_EXECFD to end up with a valid value of 0. Check
> errno when using qemu_getauxval instead of return value to handle this
> case.
> 
> Not handling this case leads to a confusing condition where the
> executable ends up as fd 0, i.e. stdin.
> 
> Signed-off-by: Vivian Wang <uwu@dram.page>
> Fixes: 0b959cf5e4cc ("linux-user: Use qemu_getauxval for AT_EXECFD")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2448
> ---
>   linux-user/main.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 7d3cf45fa9..8143a0d4b0 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -755,8 +755,9 @@ int main(int argc, char **argv, char **envp)
>       /*
>        * Manage binfmt-misc open-binary flag
>        */
> +    errno = 0;
>       execfd = qemu_getauxval(AT_EXECFD);
> -    if (execfd == 0) {
> +    if (errno != 0) {
>           execfd = open(exec_path, O_RDONLY);
>           if (execfd < 0) {
>               printf("Error while loading %s: %s\n", exec_path, strerror(errno));

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~