[Qemu-devel] [PATCH] linux-user: fix "really futimens" condition in sys_utimensat()

[Qemu-devel] [PATCH] linux-user: fix "really futimens" condition in sys_utimensat()

[Peter Maydell]

In some configurations we implement sys_utimensat() via a wrapper
that calls either futimens() or utimensat(), depending on the
arguments (to handle a case where the Linux syscall API diverges
from the glibc API). Fix a corner case in this handling:
if the syscall is passed a NULL pathname and dirfd == AT_FDCWD,
then it must fail with EFAULT. We can't handle this by passing
it to glibc utimensat() because at the libc level a NULL
pathname is failed with EINVAL, and we can't handle it by
passing to futimens() because that would fail with EBADF.
So special case it and return EFAULT directly from the wrapper.

This means that if the guest calls utimes() with a NULL pathname
and guest glibc converts that into a syscall utimensat(AT_FDCWD,
NULL, ...) then we correctly fail it with EFAULT.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 0e87157..61ea58b 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -367,10 +367,15 @@ static int sys_getcwd1(char *buf, size_t size)
 static int sys_utimensat(int dirfd, const char *pathname,
     const struct timespec times[2], int flags)
 {
-    if (pathname == NULL)
+    if (pathname == NULL) {
+        if (dirfd == AT_FDCWD) {
+            errno = EFAULT;
+            return -1;
+        }
         return futimens(dirfd, times);
-    else
+    } else {
         return utimensat(dirfd, pathname, times, flags);
+    }
 }
 #elif defined(__NR_utimensat)
 #define __NR_sys_utimensat __NR_utimensat
-- 
1.9.1

Re: [Qemu-devel] [PATCH] linux-user: fix "really futimens" condition in sys_utimensat()

[Peter Maydell]

On 15 July 2016 at 18:43, Peter Maydell <peter.maydell@linaro.org> wrote:
> In some configurations we implement sys_utimensat() via a wrapper
> that calls either futimens() or utimensat(), depending on the
> arguments (to handle a case where the Linux syscall API diverges
> from the glibc API). Fix a corner case in this handling:
> if the syscall is passed a NULL pathname and dirfd == AT_FDCWD,
> then it must fail with EFAULT. We can't handle this by passing
> it to glibc utimensat() because at the libc level a NULL
> pathname is failed with EINVAL, and we can't handle it by
> passing to futimens() because that would fail with EBADF.
> So special case it and return EFAULT directly from the wrapper.
>
> This means that if the guest calls utimes() with a NULL pathname
> and guest glibc converts that into a syscall utimensat(AT_FDCWD,
> NULL, ...) then we correctly fail it with EFAULT.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/syscall.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 0e87157..61ea58b 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -367,10 +367,15 @@ static int sys_getcwd1(char *buf, size_t size)
>  static int sys_utimensat(int dirfd, const char *pathname,
>      const struct timespec times[2], int flags)
>  {
> -    if (pathname == NULL)
> +    if (pathname == NULL) {
> +        if (dirfd == AT_FDCWD) {
> +            errno = EFAULT;
> +            return -1;
> +        }
>          return futimens(dirfd, times);
> -    else
> +    } else {
>          return utimensat(dirfd, pathname, times, flags);
> +    }
>  }
>  #elif defined(__NR_utimensat)
>  #define __NR_sys_utimensat __NR_utimensat

There turns out to be another annoying corner case here, which is
when pathname == NULL, dirfd != AT_FDCWD and the flags include
AT_SYMLINK_NOFOLLOW -- this is supposed to fail EINVAL. I'll
have a look at that next week and see whether it's best fixed
in the same patch as this case or with a followup patch...

thanks
-- PMM