diff mbox series

[v8,81/87] linux-user: Add support for statx() syscall for all platforms

Message ID 1534182832-554-82-git-send-email-aleksandar.markovic@rt-rk.com
State New
Headers show
Series Add nanoMIPS support to QEMU | expand

Commit Message

Aleksandar Markovic Aug. 13, 2018, 5:53 p.m. UTC 
From: Aleksandar Rikalo <arikalo@wavecomp.com>

Implement support for translation of system call statx(). The
implementation includes invoking other (more mature) syscalls
(from the same 'stat' family) on the host side. This way,
problems of availability of statx() on the host side are
avoided.

Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
Signed-off-by: Stefan Markovic <smarkovic@wavecomp.com>
---
 linux-user/syscall.c      | 121 +++++++++++++++++++++++++++++++++++++++++++++-
 linux-user/syscall_defs.h |  38 +++++++++++++++
 2 files changed, 158 insertions(+), 1 deletion(-)

Comments

Timothy Baldwin Aug. 20, 2018, 7:48 a.m. UTC | #1 
On 13/08/18 18:53, Aleksandar Markovic wrote:
> From: Aleksandar Rikalo <arikalo@wavecomp.com>
> 
> Implement support for translation of system call statx(). The
> implementation includes invoking other (more mature) syscalls
> (from the same 'stat' family) on the host side. This way,
> problems of availability of statx() on the host side are
> avoided.
> 
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> Signed-off-by: Stefan Markovic <smarkovic@wavecomp.com>
> ---
>   linux-user/syscall.c      | 121 +++++++++++++++++++++++++++++++++++++++++++++-
>   linux-user/syscall_defs.h |  38 +++++++++++++++
>   2 files changed, 158 insertions(+), 1 deletion(-)
> 


> +            if ((p == NULL) || (*((char *)p) == 0)) {
> +                /* By file descriptor */
> +                ret = get_errno(fstat(dirfd, &st));
> +                unlock_user(p, arg2, 0);
> +            } else if (*((char *)p) == '/') {
> +                /* Absolute pathname */
> +                ret = get_errno(stat(path(p), &st));
> +                unlock_user(p, arg2, 0);
> +            } else {
> +                if (dirfd == AT_FDCWD) {
> +                    /* Pathname relative to the current working directory */
> +                    ret = get_errno(stat(path(p), &st));
> +                    unlock_user(p, arg2, 0);
> +                } else {
> +                    /*
> +                     * Pathname relative to the directory referred to by the
> +                     * file descriptor dirfd
> +                     */
> +                    ret = get_errno(fstatat(dirfd, path(p), &st, flags));
> +                    unlock_user(p, arg2, 0);
> +                }
> +            }

This doesn't correctly handle the flags argument, it is ignored unless a 
relative path and directory file descriptor is provided. As such an 
implementation of lstat that uses statx will be broken.

Since fstatat exists since 2.6.16 this can be reduced to a call to fstatat.
Aleksandar Markovic Aug. 20, 2018, 9:45 a.m. UTC | #2 
> From: Timothy Baldwin <T.E.Baldwin99@members.leeds.ac.uk>
> Sent: Monday, August 20, 2018 9:48 AM
> 
> Subject: Re: [PATCH v8 81/87] linux-user: Add support for statx() syscall for all platforms
> 
> On 13/08/18 18:53, Aleksandar Markovic wrote:
> > From: Aleksandar Rikalo <arikalo@wavecomp.com>
> >
> > Implement support for translation of system call statx(). The
> > implementation includes invoking other (more mature) syscalls
> > (from the same 'stat' family) on the host side. This way,
> > problems of availability of statx() on the host side are
> > avoided.
> >
> > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> > Signed-off-by: Stefan Markovic <smarkovic@wavecomp.com>
> > ---
> >   linux-user/syscall.c      | 121 +++++++++++++++++++++++++++++++++++++++++++++-
> >   linux-user/syscall_defs.h |  38 +++++++++++++++
> >   2 files changed, 158 insertions(+), 1 deletion(-)
> >
> 
> 
> > +            if ((p == NULL) || (*((char *)p) == 0)) {
> > +                /* By file descriptor */
> > +                ret = get_errno(fstat(dirfd, &st));
> > +                unlock_user(p, arg2, 0);
> > +            } else if (*((char *)p) == '/') {
> > +                /* Absolute pathname */
> > +                ret = get_errno(stat(path(p), &st));
> > +                unlock_user(p, arg2, 0);
> > +            } else {
> > +                if (dirfd == AT_FDCWD) {
> > +                    /* Pathname relative to the current working directory */
> > +                    ret = get_errno(stat(path(p), &st));
> > +                    unlock_user(p, arg2, 0);
> > +                } else {
> > +                    /*
> > +                     * Pathname relative to the directory referred to by the
> > +                     * file descriptor dirfd
> > +                     */
> > +                    ret = get_errno(fstatat(dirfd, path(p), &st, flags));
> > +                    unlock_user(p, arg2, 0);
> > +                }
> > +            }
> 
> This doesn't correctly handle the flags argument, it is ignored unless a
> relative path and directory file descriptor is provided.

Hi, Timothy.

Agreed. The patch generally needs certain improvements wrt handling original statx() arguments. I would certianly add handling the mask argument.

> As such an implementation of lstat that uses statx will be broken.
> 
> Since fstatat exists since 2.6.16 this can be reduced to a call to fstatat.

I am not sure what you meant here. I think QEMU lstat() implementation does not use statx(). This implementation of statx() uses hosts's statx(), and, as a fallback, host's fstat(), stat(), and fstatat().

I would like to add that I think this patch should be submitted separately, out of this series.

Yours,
Aleksandar
Timothy Baldwin Aug. 29, 2018, 3:38 p.m. UTC | #3 
On Mon, 20 Aug 2018, at 10:45 AM, Aleksandar Markovic wrote:

> > As such an implementation of lstat that uses statx will be broken.
> > 
> > Since fstatat exists since 2.6.16 this can be reduced to a call to fstatat.
> 
> I am not sure what you meant here. I think QEMU lstat() implementation 
> does not use statx(). This implementation of statx() uses hosts's 
> statx(), and, as a fallback, host's fstat(), stat(), and fstatat().

I was referring to an  implementation of lstat in a C library that is running on QEMU linux user.

> 
> I would like to add that I think this patch should be submitted 
> separately, out of this series.
> 
> Yours,
> Aleksandar
>
diff mbox series

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index bced9b8..f1b6d71 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8002,7 +8002,8 @@  abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     abi_long ret;
 #if defined(TARGET_NR_stat) || defined(TARGET_NR_stat64) \
     || defined(TARGET_NR_lstat) || defined(TARGET_NR_lstat64) \
-    || defined(TARGET_NR_fstat) || defined(TARGET_NR_fstat64)
+    || defined(TARGET_NR_fstat) || defined(TARGET_NR_fstat64) \
+    || defined(TARGET_NR_statx)
     struct stat st;
 #endif
 #if defined(TARGET_NR_statfs) || defined(TARGET_NR_statfs64) \
@@ -10025,6 +10026,124 @@  abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         }
         break;
 #endif
+#if defined(TARGET_NR_statx)
+    case TARGET_NR_statx:
+        {
+            struct target_statx *target_stx;
+            int dirfd = tswap32(arg1);
+            int flags = tswap32(arg3);
+
+            p = lock_user_string(arg2);
+            if (p == NULL) {
+                goto efault;
+            }
+#if defined(__NR_statx)
+            {
+                /* We assume that struct statx is arhitecture independent */
+                struct target_statx host_stx;
+                int mask = tswap32(arg4);
+
+                ret = get_errno(syscall(__NR_statx, dirfd, p, flags, mask,
+                                        &host_stx));
+                if (!is_error(ret)) {
+                    unlock_user(p, arg2, 0);
+                    if (!lock_user_struct(VERIFY_WRITE, target_stx, arg5, 0)) {
+                        goto efault;
+                    }
+                    memset(target_stx, 0, sizeof(*target_stx));
+
+                    __put_user(host_stx.stx_mask, &target_stx->stx_mask);
+                    __put_user(host_stx.stx_blksize, &target_stx->stx_blksize);
+                    __put_user(host_stx.stx_attributes,
+                               &target_stx->stx_attributes);
+                    __put_user(host_stx.stx_nlink, &target_stx->stx_nlink);
+                    __put_user(host_stx.stx_uid, &target_stx->stx_uid);
+                    __put_user(host_stx.stx_gid, &target_stx->stx_gid);
+                    __put_user(host_stx.stx_mode, &target_stx->stx_mode);
+                    __put_user(host_stx.stx_ino, &target_stx->stx_ino);
+                    __put_user(host_stx.stx_size, &target_stx->stx_size);
+                    __put_user(host_stx.stx_blocks, &target_stx->stx_blocks);
+                    __put_user(host_stx.stx_attributes_mask,
+                               &target_stx->stx_attributes_mask);
+                    __put_user(host_stx.stx_atime.tv_sec,
+                               &target_stx->stx_atime.tv_sec);
+                    __put_user(host_stx.stx_atime.tv_nsec,
+                               &target_stx->stx_atime.tv_nsec);
+                    __put_user(host_stx.stx_btime.tv_sec,
+                               &target_stx->stx_atime.tv_sec);
+                    __put_user(host_stx.stx_btime.tv_nsec,
+                               &target_stx->stx_atime.tv_nsec);
+                    __put_user(host_stx.stx_ctime.tv_sec,
+                               &target_stx->stx_atime.tv_sec);
+                    __put_user(host_stx.stx_ctime.tv_nsec,
+                               &target_stx->stx_atime.tv_nsec);
+                    __put_user(host_stx.stx_mtime.tv_sec,
+                               &target_stx->stx_atime.tv_sec);
+                    __put_user(host_stx.stx_mtime.tv_nsec,
+                               &target_stx->stx_atime.tv_nsec);
+                    __put_user(host_stx.stx_rdev_major,
+                               &target_stx->stx_rdev_major);
+                    __put_user(host_stx.stx_rdev_minor,
+                               &target_stx->stx_rdev_minor);
+                    __put_user(host_stx.stx_dev_major,
+                               &target_stx->stx_dev_major);
+                    __put_user(host_stx.stx_dev_minor,
+                               &target_stx->stx_dev_minor);
+                }
+
+                if (ret != TARGET_ENOSYS) {
+                    break;
+                }
+            }
+#endif
+            if ((p == NULL) || (*((char *)p) == 0)) {
+                /* By file descriptor */
+                ret = get_errno(fstat(dirfd, &st));
+                unlock_user(p, arg2, 0);
+            } else if (*((char *)p) == '/') {
+                /* Absolute pathname */
+                ret = get_errno(stat(path(p), &st));
+                unlock_user(p, arg2, 0);
+            } else {
+                if (dirfd == AT_FDCWD) {
+                    /* Pathname relative to the current working directory */
+                    ret = get_errno(stat(path(p), &st));
+                    unlock_user(p, arg2, 0);
+                } else {
+                    /*
+                     * Pathname relative to the directory referred to by the
+                     * file descriptor dirfd
+                     */
+                    ret = get_errno(fstatat(dirfd, path(p), &st, flags));
+                    unlock_user(p, arg2, 0);
+                }
+            }
+
+            if (!is_error(ret)) {
+                if (!lock_user_struct(VERIFY_WRITE, target_stx, arg5, 0)) {
+                    goto efault;
+                }
+                memset(target_stx, 0, sizeof(*target_stx));
+                __put_user(major(st.st_dev), &target_stx->stx_dev_major);
+                __put_user(minor(st.st_dev), &target_stx->stx_dev_minor);
+                __put_user(st.st_ino, &target_stx->stx_ino);
+                __put_user(st.st_mode, &target_stx->stx_mode);
+                __put_user(st.st_uid, &target_stx->stx_uid);
+                __put_user(st.st_gid, &target_stx->stx_gid);
+                __put_user(st.st_nlink, &target_stx->stx_nlink);
+                __put_user(major(st.st_rdev), &target_stx->stx_rdev_major);
+                __put_user(minor(st.st_rdev), &target_stx->stx_rdev_minor);
+                __put_user(st.st_size, &target_stx->stx_size);
+                __put_user(st.st_blksize, &target_stx->stx_blksize);
+                __put_user(st.st_blocks, &target_stx->stx_blocks);
+                __put_user(st.st_atime, &target_stx->stx_atime.tv_sec);
+                __put_user(st.st_mtime, &target_stx->stx_mtime.tv_sec);
+                __put_user(st.st_ctime, &target_stx->stx_ctime.tv_sec);
+                unlock_user_struct(target_stx, arg5, 1);
+            }
+        }
+        break;
+#endif
 #ifdef TARGET_NR_olduname
     case TARGET_NR_olduname:
         goto unimplemented;
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index abf94b8..34cc6e0 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2530,4 +2530,42 @@  struct target_user_cap_data {
 /* Return size of the log buffer */
 #define TARGET_SYSLOG_ACTION_SIZE_BUFFER   10
 
+struct target_statx_timestamp {
+   int64_t tv_sec;
+   uint32_t tv_nsec;
+   int32_t __reserved;
+};
+
+struct target_statx {
+   /* 0x00 */
+   uint32_t stx_mask;       /* What results were written [uncond] */
+   uint32_t stx_blksize;    /* Preferred general I/O size [uncond] */
+   uint64_t stx_attributes; /* Flags conveying information about the file */
+   /* 0x10 */
+   uint32_t stx_nlink;      /* Number of hard links */
+   uint32_t stx_uid;        /* User ID of owner */
+   uint32_t stx_gid;        /* Group ID of owner */
+   uint16_t stx_mode;       /* File mode */
+   uint16_t __spare0[1];
+   /* 0x20 */
+   uint64_t stx_ino;        /* Inode number */
+   uint64_t stx_size;       /* File size */
+   uint64_t stx_blocks;     /* Number of 512-byte blocks allocated */
+   uint64_t stx_attributes_mask; /* Mask to show what's supported in
+                                    stx_attributes */
+   /* 0x40 */
+   struct target_statx_timestamp  stx_atime;  /* Last access time */
+   struct target_statx_timestamp  stx_btime;  /* File creation time */
+   struct target_statx_timestamp  stx_ctime;  /* Last attribute change time */
+   struct target_statx_timestamp  stx_mtime;  /* Last data modification time */
+   /* 0x80 */
+   uint32_t stx_rdev_major;   /* Device ID of special file [if bdev/cdev] */
+   uint32_t stx_rdev_minor;
+   uint32_t stx_dev_major; /* ID of device containing file [uncond] */
+   uint32_t stx_dev_minor;
+   /* 0x90 */
+   uint64_t __spare2[14];  /* Spare space for future expansion */
+   /* 0x100 */
+};
+
 #endif