On Tue, Jun 19, 2018 at 10:42:50AM +0100, Marc Zyngier wrote:
> The current behaviour of the compat ioctls is a bit odd.
> We provide a compat_ioctl method when KVM_COMPAT is set, and NULL
> otherwise. But NULL means that the normal, non-compat ioctl should
> be used directly for compat tasks, and there is no way to actually
> prevent a compat task from issueing KVM ioctls.
> 
> This patch changes this behaviour, by always registering a compat_ioctl
> method, even if KVM_COMPAT is not selected. In that case, the callback
> will always return -EINVAL.
> 
> Reported-by: Mark Rutland <mark.rutl...@arm.com>
> Signed-off-by: Marc Zyngier <marc.zyng...@arm.com>

I virt/kvm/Kconfig we have:

config KVM_COMPAT
       def_bool y
       depends on KVM && COMPAT && !S390

... and in arch/s390 we have COMPAT support, so does this potentially break
anything there?

Thanks,
Mark.

> ---
>  virt/kvm/kvm_main.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index ada21f47f22b..8b47507faab5 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -116,6 +116,11 @@ static long kvm_vcpu_ioctl(struct file *file, unsigned 
> int ioctl,
>  #ifdef CONFIG_KVM_COMPAT
>  static long kvm_vcpu_compat_ioctl(struct file *file, unsigned int ioctl,
>                                 unsigned long arg);
> +#define KVM_COMPAT(c)        .compat_ioctl   = (c)
> +#else
> +static long kvm_no_compat_ioctl(struct file *file, unsigned int ioctl,
> +                             unsigned long arg) { return -EINVAL; }
> +#define KVM_COMPAT(c)        .compat_ioctl   = kvm_no_compat_ioctl
>  #endif
>  static int hardware_enable_all(void);
>  static void hardware_disable_all(void);
> @@ -2396,11 +2401,9 @@ static int kvm_vcpu_release(struct inode *inode, 
> struct file *filp)
>  static struct file_operations kvm_vcpu_fops = {
>       .release        = kvm_vcpu_release,
>       .unlocked_ioctl = kvm_vcpu_ioctl,
> -#ifdef CONFIG_KVM_COMPAT
> -     .compat_ioctl   = kvm_vcpu_compat_ioctl,
> -#endif
>       .mmap           = kvm_vcpu_mmap,
>       .llseek         = noop_llseek,
> +     KVM_COMPAT(kvm_vcpu_compat_ioctl),
>  };
>  
>  /*
> @@ -2824,10 +2827,8 @@ static int kvm_device_release(struct inode *inode, 
> struct file *filp)
>  
>  static const struct file_operations kvm_device_fops = {
>       .unlocked_ioctl = kvm_device_ioctl,
> -#ifdef CONFIG_KVM_COMPAT
> -     .compat_ioctl = kvm_device_ioctl,
> -#endif
>       .release = kvm_device_release,
> +     KVM_COMPAT(kvm_device_ioctl),
>  };
>  
>  struct kvm_device *kvm_device_from_filp(struct file *filp)
> @@ -3165,10 +3166,8 @@ static long kvm_vm_compat_ioctl(struct file *filp,
>  static struct file_operations kvm_vm_fops = {
>       .release        = kvm_vm_release,
>       .unlocked_ioctl = kvm_vm_ioctl,
> -#ifdef CONFIG_KVM_COMPAT
> -     .compat_ioctl   = kvm_vm_compat_ioctl,
> -#endif
>       .llseek         = noop_llseek,
> +     KVM_COMPAT(kvm_vm_compat_ioctl),
>  };
>  
>  static int kvm_dev_ioctl_create_vm(unsigned long type)
> @@ -3259,8 +3258,8 @@ static long kvm_dev_ioctl(struct file *filp,
>  
>  static struct file_operations kvm_chardev_ops = {
>       .unlocked_ioctl = kvm_dev_ioctl,
> -     .compat_ioctl   = kvm_dev_ioctl,
>       .llseek         = noop_llseek,
> +     KVM_COMPAT(kvm_dev_ioctl),
>  };
>  
>  static struct miscdevice kvm_dev = {
> -- 
> 2.17.1
> 
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to