[PATCH 6/8] kvm: decode the argument for KVM_{SET, GET}_REGS ioctl command

[Dmitry V. Levin]

On Sat, Dec 02, 2017 at 02:25:37AM +0300, Dmitry V. Levin wrote:
> On Sat, Dec 02, 2017 at 04:05:30AM +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.
> 
> Could you please add linux/x32/arch_kvm.c containing just one line,
> #include "x86_64/arch_kvm.c"?

btw, all these new files has to be listed in the main Makefile.am's EXTRA_DIST.

> > * kvm.c (kvm_ioctl_decode_regs): New function.
> > (kvm_ioctl) <KVM_SET_REGS, KVM_GET_REGS>: Use it.
> > (top-level): Include "arch_kvm.c".
> > 
> > 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.
> > 
> > Signed-off-by: Masatake YAMATO <yamato at redhat.com>
> > ---
> >  configure.ac            |  2 ++
> >  kvm.c                   | 28 ++++++++++++++++++++++++++--
> >  linux/arch_kvm.c        |  7 +++++++
> >  linux/x86_64/arch_kvm.c | 36 ++++++++++++++++++++++++++++++++++++
> >  4 files changed, 71 insertions(+), 2 deletions(-)
> >  create mode 100644 linux/arch_kvm.c
> >  create mode 100644 linux/x86_64/arch_kvm.c
> > 
> > diff --git a/configure.ac b/configure.ac
> > index fa451d84..93d4bd73 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -541,6 +541,8 @@ AC_CHECK_TYPES([struct statfs64], [
> >  
> >  AC_CHECK_TYPES([struct blk_user_trace_setup],,, [#include <linux/blktrace_api.h>])
> >  
> > +AC_CHECK_TYPES([struct kvm_regs],,, [#include <linux/kvm.h>])
> > +
> >  AC_CHECK_HEADERS([linux/btrfs.h], [
> >  	AC_CHECK_MEMBERS(m4_normalize([
> >  		struct btrfs_ioctl_feature_flags.compat_flags,
> > diff --git a/kvm.c b/kvm.c
> > index 19779c84..a4bab9d4 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)
> > @@ -63,16 +64,39 @@ kvm_ioctl_set_user_memory_region(struct tcb *const tcp, const kernel_ulong_t arg
> >  	return RVAL_IOCTL_DECODED;
> >  }
> >  
> > +static int
> > +kvm_ioctl_decode_regs(struct tcb *const tcp, const unsigned int code, const kernel_ulong_t arg)
> > +{
> > +#ifdef HAVE_STRUCT_KVM_REGS
> > +	struct kvm_regs regs;
> 
> Looks like struct kvm_regs was in linux/kvm.h from the beginning (kernel
> commit v2.6.20-rc1~15^2~39), so the check might be redundant after all.
> However, if you want to keep it, then ...

Starting with kernel commit v2.6.25-rc1~1138^2~118 struct kvm_regs
is arch-specific and is no longer defined in linux/kvm.h itself,
so a check is needed to avoid compile time errors on those architectures
that provide no definition of struct kvm_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);
> > +	return RVAL_IOCTL_DECODED;
> > +#else
> > +	return RVAL_DECODED;
> > +#endif
> > +}
> > +
> >  int
> >  kvm_ioctl(struct tcb *const tcp, const unsigned int code, const kernel_ulong_t arg)
> >  {
> >  	switch (code) {
> >  	case KVM_CREATE_VCPU:
> >  		return kvm_ioctl_create_vcpu(tcp, arg);
> > -	case KVM_CREATE_VM:
> > -		return RVAL_DECODED | RVAL_FD;
> >  	case KVM_SET_USER_MEMORY_REGION:
> >  		return kvm_ioctl_set_user_memory_region(tcp, arg);
> > +	case KVM_SET_REGS:
> > +	case KVM_GET_REGS:
> > +		return kvm_ioctl_decode_regs(tcp, code, arg);

On those architectures that provide no definition of struct kvm_regs
both KVM_SET_REGS and KVM_GET_REGS are still defined to an expression that
doesn't compile.  Looks like HAVE_STRUCT_KVM_REGS guard is necessary here.


-- 
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/20171202/8b03d073/attachment.bin>