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.rutland@xxxxxxx>
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
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
>
