Re: [PATCH v7 2/2] linux-user: add strace support for openat2

[Laurent Vivier <laurent@vivier.eu>]

Le 23/09/2024 à 21:37, Michael Vogt a écrit :
> This commit adds support for the `openat2()` to `QEMU_STRACE`. It
> will use the `openat2.h` header if available to create user
> readable flags for the `resolve` argument but does not require
> the header otherwise.
> 
> It also makes `copy_struct_from_user()` available via `qemu.h`
> and `open_how_ver0` via `syscall_defs.h` so that strace.c can use
> them.
> 
> Signed-off-by: Michael Vogt <mvogt@redhat.com>
> ---
>   linux-user/qemu.h         |  9 +++++++++
>   linux-user/strace.c       | 40 +++++++++++++++++++++++++++++++++++++++
>   linux-user/strace.list    |  3 +++
>   linux-user/syscall.c      |  8 +-------
>   linux-user/syscall_defs.h |  5 +++++
>   meson.build               |  1 +
>   6 files changed, 59 insertions(+), 7 deletions(-)
> 
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 2e90a97175..98ad848ab2 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -313,6 +313,15 @@ static inline bool access_ok(CPUState *cpu, int type,
>   int copy_from_user(void *hptr, abi_ulong gaddr, ssize_t len);
>   int copy_to_user(abi_ulong gaddr, void *hptr, ssize_t len);
>   
> +/*
> + * copy_struct_from_user() copies a target struct to a host struct, in
> + * a way that guarantees backwards-compatibility for struct syscall
> + * arguments.
> + *
> + * Similar to kernels uaccess.h:copy_struct_from_user()
> + */
> +int copy_struct_from_user(void *dst, size_t ksize, abi_ptr src, size_t usize);
> +
>   /* Functions for accessing guest memory.  The tget and tput functions
>      read/write single values, byteswapping as necessary.  The lock_user function
>      gets a pointer to a contiguous area of guest memory, but does not perform
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index b4d1098170..77d5108e5d 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -13,6 +13,9 @@
>   #include <linux/if_packet.h>
>   #include <linux/in6.h>
>   #include <linux/netlink.h>
> +#ifdef HAVE_OPENAT2_H
> +#include <linux/openat2.h>
> +#endif
>   #include <sched.h>
>   #include "qemu.h"
>   #include "user-internals.h"
> @@ -1063,6 +1066,18 @@ UNUSED static const struct flags open_flags[] = {
>       FLAG_END,
>   };
>   
> +UNUSED static const struct flags openat2_resolve_flags[] = {
> +#ifdef HAVE_OPENAT2_H
> +    FLAG_GENERIC(RESOLVE_NO_XDEV),
> +    FLAG_GENERIC(RESOLVE_NO_MAGICLINKS),
> +    FLAG_GENERIC(RESOLVE_NO_SYMLINKS),
> +    FLAG_GENERIC(RESOLVE_BENEATH),
> +    FLAG_GENERIC(RESOLVE_IN_ROOT),
> +    FLAG_GENERIC(RESOLVE_CACHED),
> +#endif
> +    FLAG_END,
> +};
> +
>   UNUSED static const struct flags mount_flags[] = {
>   #ifdef MS_BIND
>       FLAG_GENERIC(MS_BIND),
> @@ -3483,6 +3498,31 @@ print_openat(CPUArchState *cpu_env, const struct syscallname *name,
>   }
>   #endif
>   
> +#ifdef TARGET_NR_openat2
> +static void
> +print_openat2(CPUArchState *cpu_env, const struct syscallname *name,
> +              abi_long arg0, abi_long arg1, abi_long arg2,
> +              abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    struct open_how_ver0 how = {0};
> +
> +    print_syscall_prologue(name);
> +    print_at_dirfd(arg0, 0);
> +    print_string(arg1, 0);
> +    if (copy_struct_from_user(&how, sizeof(how), arg2, arg3) == 0) {

I think you need also to tswap64() all the fields of how.

> +        print_open_flags(how.flags, 0);
> +        if (how.flags & TARGET_O_CREAT) {
> +            print_file_mode(how.mode, 0);
> +        }
> +        print_flags(openat2_resolve_flags, how.resolve, 0);
> +    } else {
> +        print_pointer(arg2, 0);
> +    }
> +    print_raw_param("size=" TARGET_ABI_FMT_lu, arg3, 1);

Why the "size="?
You can write: print_raw_param(TARGET_ABI_FMT_lu, arg3, 1);

Thanks,
Laurent