[PATCH v4 2/4] kvm: decode the argument for KVM_{SET, GET}_REGS ioctl command

Dmitry V. Levin ldv at altlinux.org
Thu Dec 7 12:57:17 UTC 2017 

On Mon, Dec 04, 2017 at 10:08:15PM +0900, Masatake YAMATO wrote:
> * configure.ac (AC_CHECK_TYPES): Add struct kvm_regs.
> * linux/arck_kvm.c: New file.
> * linux/x86_64/arch_kvm.c: New file.
> * linux/x32/arch_kvm.c: New files.
> * Makefile.am (EXTRA_DIST): Add the new files.
> * kvm.c Include "arch_kvm.c".
> (kvm_ioctl_decode_regs): New function.
> (kvm_ioctl) <KVM_SET_REGS, KVM_GET_REGS>: Use it.
> 
> Changes in v2:
> * Decode only if struct kvm_regs is available.
> * Introduce files in which arch-specific and generic stub decoders
>   are defined. Use them from kvm_ioctl_decode_regs.
> * Use umove_or_printaddr instead of umove.
> 
>   All items are suggested by ldv.
> 
> Change in v3:
> * Don't check the existence of struct kvm_regs.
>   We assume we can rely on struct kvm_regs being
>   available. Suggested by ldv.
> * Add linux/x32/arch_kvm.c.
> * Add new files to EXTRA_DIST.
> 
>   All items are suggested by ldv.
> 
> Highlights in v4:
> * Guard the case label with ifdef HAVE_STRUCT_KVM_REGS.
> * Sort when adding a new file to the lists in Makefile.am.
> * Sort when adding a new type to the AC_CHECK_TYPES.
> 
>   All items are suggested by ldv.
> 
> Signed-off-by: Masatake YAMATO <yamato at redhat.com>
> ---
>  Makefile.am             |  3 +++
>  configure.ac            |  5 ++++-
>  kvm.c                   | 24 ++++++++++++++++++++++++
>  linux/arch_kvm.c        |  9 +++++++++
>  linux/x32/arch_kvm.c    |  1 +
>  linux/x86_64/arch_kvm.c | 38 ++++++++++++++++++++++++++++++++++++++
>  6 files changed, 79 insertions(+), 1 deletion(-)
>  create mode 100644 linux/arch_kvm.c
>  create mode 100644 linux/x32/arch_kvm.c
>  create mode 100644 linux/x86_64/arch_kvm.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index 60376056..29389995 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -424,6 +424,7 @@ EXTRA_DIST =				\
>  	linux/arc/set_error.c		\
>  	linux/arc/set_scno.c		\
>  	linux/arc/syscallent.h		\
> +	linux/arch_kvm.c		\
>  	linux/arch_regs.h		\
>  	linux/arch_sigreturn.c		\
>  	linux/arm/arch_regs.c		\
> @@ -805,6 +806,7 @@ EXTRA_DIST =				\
>  	linux/unix_diag.h		\
>  	linux/userent.h			\
>  	linux/userent0.h		\
> +	linux/x32/arch_kvm.c		\
>  	linux/x32/arch_regs.c		\
>  	linux/x32/arch_regs.h		\
>  	linux/x32/arch_rt_sigframe.c	\
> @@ -824,6 +826,7 @@ EXTRA_DIST =				\
>  	linux/x32/syscallent.h		\
>  	linux/x32/syscallent1.h		\
>  	linux/x32/userent.h		\
> +	linux/x86_64/arch_kvm.c		\
>  	linux/x86_64/arch_regs.c	\
>  	linux/x86_64/arch_regs.h	\
>  	linux/x86_64/arch_rt_sigframe.c	\
> diff --git a/configure.ac b/configure.ac
> index dad995c4..5103d479 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -541,7 +541,10 @@ AC_CHECK_TYPES([struct statfs64], [
>  
>  AC_CHECK_TYPES([struct blk_user_trace_setup],,, [#include <linux/blktrace_api.h>])
>  
> -AC_CHECK_TYPES([struct kvm_userspace_memory_region],,, [#include <linux/kvm.h>])
> +AC_CHECK_TYPES(m4_normalize([
> +	struct kvm_regs
> +	struct kvm_userspace_memory_region,
> +]),,, [#include <linux/kvm.h>])
>  
>  AC_CHECK_HEADERS([linux/btrfs.h], [
>  	AC_CHECK_MEMBERS(m4_normalize([
> diff --git a/kvm.c b/kvm.c
> index f0531c6b..db705734 100644
> --- a/kvm.c
> +++ b/kvm.c
> @@ -33,6 +33,7 @@
>  #ifdef HAVE_LINUX_KVM_H
>  # include <linux/kvm.h>
>  # include "print_fields.h"
> +# include "arch_kvm.c"
>  
>  static int
>  kvm_ioctl_create_vcpu(struct tcb *const tcp, const kernel_ulong_t arg)
> @@ -65,6 +66,23 @@ kvm_ioctl_set_user_memory_region(struct tcb *const tcp, const kernel_ulong_t arg
>  }
>  # endif /* HAVE_STRUCT_KVM_USERSPACE_MEMORY_REGION */
>  
> +# ifdef HAVE_STRUCT_KVM_REGS
> +static int
> +kvm_ioctl_decode_regs(struct tcb *const tcp, const unsigned int code, const kernel_ulong_t arg)
> +{
> +	struct kvm_regs regs;
> +
> +	if (code == KVM_GET_REGS && entering(tcp))
> +		return 0;
> +
> +	tprints(", ");
> +	if (umove_or_printaddr(tcp, arg, &regs))
> +		return RVAL_DECODED;
> +
> +	arch_print_kvm_regs(tcp, arg, &regs);

There is an output issue here: once the decoder has printed something,
it shouldn't return RVAL_DECODED.

What you probably want is this:

	if (!umove_or_printaddr(tcp, arg, &regs))
		arch_print_kvm_regs(tcp, arg, &regs);

The next patch that adds kvm_sregs decoding has the same issue.

> +	return RVAL_IOCTL_DECODED;
> +}
> +# endif /* HAVE_STRUCT_KVM_REGS */
>  
>  int
>  kvm_ioctl(struct tcb *const tcp, const unsigned int code, const kernel_ulong_t arg)
> @@ -78,6 +96,12 @@ kvm_ioctl(struct tcb *const tcp, const unsigned int code, const kernel_ulong_t a
>  		return kvm_ioctl_set_user_memory_region(tcp, arg);
>  # endif
>  
> +# ifdef HAVE_STRUCT_KVM_REGS
> +	case KVM_SET_REGS:
> +	case KVM_GET_REGS:
> +		return kvm_ioctl_decode_regs(tcp, code, arg);
> +# endif
> +
>  	case KVM_CREATE_VM:
>  		return RVAL_DECODED | RVAL_FD;
>  	case KVM_RUN:
> diff --git a/linux/arch_kvm.c b/linux/arch_kvm.c
> new file mode 100644
> index 00000000..d1292a5b
> --- /dev/null
> +++ b/linux/arch_kvm.c
> @@ -0,0 +1,9 @@
> +#ifdef HAVE_STRUCT_KVM_REGS
> +static void
> +arch_print_kvm_regs(struct tcb *const tcp,
> +		    const kernel_ulong_t addr,
> +		    const struct kvm_regs *const regs)
> +{
> +	printaddr(addr);
> +}
> +#endif	/* HAVE_STRUCT_KVM_REGS */
> diff --git a/linux/x32/arch_kvm.c b/linux/x32/arch_kvm.c
> new file mode 100644
> index 00000000..e8e982ee
> --- /dev/null
> +++ b/linux/x32/arch_kvm.c
> @@ -0,0 +1 @@
> +#include "x86_64/arch_kvm.c"

I've tested that new parser works on x86 as well,
so please add linux/i386/arch_kvm.c, too.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20171207/a1310bb2/attachment.bin>

More information about the Strace-devel mailing list