On 2019-11-14 12:12, Paolo Bonzini wrote:
On 14/11/19 09:20, Christian Borntraeger wrote:
On 14.11.19 09:15, Marc Zyngier wrote:
On Wed, 13 Nov 2019 21:23:07 +0000
Peter Maydell <peter.maydell@xxxxxxxxxx> wrote:
On Wed, 13 Nov 2019 at 18:44, Christian Borntraeger
<borntraeger@xxxxxxxxxx> wrote:
On 13.11.19 17:05, Marc Zyngier wrote:
On a system without KVM_COMPAT, we prevent IOCTLs from being
issued
by a compat task. Although this prevents most silly things from
happening, it can still confuse a 32bit userspace that is able
to open the kvm device (the qemu test suite seems to be pretty
mad with this behaviour).
Take a more radical approach and return a -ENODEV to the compat
task.
Do we still need compat_ioctl if open never succeeds?
I wondered about that, but presumably you could use
fd-passing, or just inheriting open fds across exec(),
to open the fd in a 64-bit process and then hand it off
to a 32-bit process to call the ioctl with. (That's
probably only something you'd do if you were
deliberately playing silly games, of course, but
preventing silly games is useful as it makes it
easier to reason about kernel behaviour.)
This was exactly my train of thoughts, which I should have made
clear
in the commit log. Thanks Peter for reading my mind! ;-)
Makes sense. Looks like this is already in kvm/master so we cannot
improve
the commit message easily any more. Hopefully we will not forget :-)
A comment in the code would probably be better than the commit
message,
to not forget stuff like this. (Hint! :))
Hint received. How about this?
M.
From 34bfc68752253c3da3e59072b137d1a4a85bc005 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@xxxxxxxxxx>
Date: Thu, 14 Nov 2019 13:17:39 +0000
Subject: [PATCH] KVM: Add a comment describing the /dev/kvm no_compat
handling
Add a comment explaining the rational behind having both
no_compat open and ioctl callbacks to fend off compat tasks.
Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
---
virt/kvm/kvm_main.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1243e48dc717..722f2b1d4672 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -120,6 +120,13 @@ static long kvm_vcpu_compat_ioctl(struct file
*file, unsigned int ioctl,
unsigned long arg);
#define KVM_COMPAT(c) .compat_ioctl = (c)
#else
+/*
+ * For architectures that don't implement a compat infrastructure,
+ * adopt a double line of defense:
+ * - Prevent a compat task from opening /dev/kvm
+ * - If the open has been done by a 64bit task, and the KVM fd
+ * passed to a compat task, let the ioctls fail.
+ */
static long kvm_no_compat_ioctl(struct file *file, unsigned int ioctl,
unsigned long arg) { return -EINVAL; }
--
2.20.1
--
Jazz is not dead. It just smells funny...
