Re: [PATCH 1/2] linux-user: Add thunk argument types for SIOCGSTAMP and SIOCGSTAMPNS

[Laurent Vivier]

Le 12/06/2020 à 18:40, Filip Bozuta a écrit :
> From: Filip Bozuta <Filip.Bozuta@syrmia.com>
> 
> Socket ioctls SIOCGSTAMP and SIOCGSTAMPNS, used for timestamping the socket
> connection, are defined in file "ioctls.h" differently from other ioctls.
> The reason for this difference is explained in the comments above their 
> definition.
> These ioctls didn't have defined thunk argument types before changes from this
> patch. They have special handling functions ("do_ioctl_SIOCGSTAMP" and
> "do_ioctl_SIOCGSTAMPNS") that take care of setting values for approppriate 
> argument
> types (struct timeval and struct timespec) and thus no thunk argument types 
> were
> needed for their implementation. But this patch adds those argument type 
> definitions
> in file "syscall_types.h" and "ioctls.h" as it is needed for printing 
> arguments
> of these ioctls with strace.
> 
> Implementation notes:
> 
>     There are two variants of these ioctls: SIOCGSTAMP_OLD/SIOCGSTAM_NEW and
>     SIOCGSTAMPNS_OLD/SIOCGSTAMPNS_NEW. One is the old existing definition and 
> the
>     other is the 2038 safe variant used for 32-bit architectures. These 
> variants
>     use types "struct timeval/timeval64" and "struct timespec/timespec64" 
> respectively.
>     That is the reason why corresponding structure definitions were added in 
> file
>     "syscall_types.h". STRUCT_timeval definition was already inside the file 
> as
>     it is used by another implemented ioctl.
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/ioctls.h        | 12 ++++++++----
>  linux-user/syscall_types.h | 12 ++++++++++++
>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> index 0defa1d8c1..68d43f71cc 100644
> --- a/linux-user/ioctls.h
> +++ b/linux-user/ioctls.h
> @@ -279,13 +279,17 @@
>     * FIXME: create a macro to define this kind of entry
>     */
>    { TARGET_SIOCGSTAMP_OLD, TARGET_SIOCGSTAMP_OLD,
> -    "SIOCGSTAMP_OLD", IOC_R, do_ioctl_SIOCGSTAMP },
> +    "SIOCGSTAMP_OLD", IOC_R, do_ioctl_SIOCGSTAMP,
> +    { MK_PTR(MK_STRUCT(STRUCT_timeval)) } },
>    { TARGET_SIOCGSTAMPNS_OLD, TARGET_SIOCGSTAMPNS_OLD,
> -    "SIOCGSTAMPNS_OLD", IOC_R, do_ioctl_SIOCGSTAMPNS },
> +    "SIOCGSTAMPNS_OLD", IOC_R, do_ioctl_SIOCGSTAMPNS,
> +    { MK_PTR(MK_STRUCT(STRUCT_timespec)) } },
>    { TARGET_SIOCGSTAMP_NEW, TARGET_SIOCGSTAMP_NEW,
> -    "SIOCGSTAMP_NEW", IOC_R, do_ioctl_SIOCGSTAMP },
> +    "SIOCGSTAMP_NEW", IOC_R, do_ioctl_SIOCGSTAMP,
> +    { MK_PTR(MK_STRUCT(STRUCT_timeval64)) } },
>    { TARGET_SIOCGSTAMPNS_NEW, TARGET_SIOCGSTAMPNS_NEW,
> -    "SIOCGSTAMPNS_NEW", IOC_R, do_ioctl_SIOCGSTAMPNS },
> +    "SIOCGSTAMPNS_NEW", IOC_R, do_ioctl_SIOCGSTAMPNS,
> +    { MK_PTR(MK_STRUCT(STRUCT_timespec64)) } },
>  
>    IOCTL(RNDGETENTCNT, IOC_R, MK_PTR(TYPE_INT))
>    IOCTL(RNDADDTOENTCNT, IOC_W, MK_PTR(TYPE_INT))
> diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
> index 4e12c1661e..a5ad5a9ddc 100644
> --- a/linux-user/syscall_types.h
> +++ b/linux-user/syscall_types.h
> @@ -137,10 +137,22 @@ STRUCT(snd_timer_params,
>         TYPE_INT, /* filter */
>         MK_ARRAY(TYPE_CHAR, 60)) /* reserved */
>  
> +STRUCT(timeval,
> +       TYPE_LONG, /* tv_sec */
> +       TYPE_LONG) /* tv_usec */

You have to manage the case when tv_usec is int.
See linux-user/syscall_defs.h, target_timeval (and
target__kernel_sock_timeval)

> +
> +STRUCT(timeval64,
> +       TYPE_LONGLONG, /* tv_sec */
> +       TYPE_LONGLONG) /* tv_usec */

perhaps you could call this "target__kernel_sock_timeval" as it is in
linux-user/syscall_defs.h.

> +
>  STRUCT(timespec,
>         TYPE_LONG, /* tv_sec */
>         TYPE_LONG) /* tv_nsec */
>  
> +STRUCT(timespec64,
> +       TYPE_LONGLONG, /* tv_sec */
> +       TYPE_LONGLONG) /* tv_nsec */
> +

ditto: target__kernel_timespec

>  STRUCT(snd_timer_status,
>         MK_STRUCT(STRUCT_timespec), /* tstamp */
>         TYPE_INT, /* resolution */
>