commit cfa39381173d5f969daf43582c95ad679189cbc9 upstream.
kvm_ioctl_create_device() does the following:
1. creates a device that holds a reference to the VM object (with a borrowed reference, the VM's refcount has not been bumped yet) 2. initializes the device 3. transfers the reference to the device to the caller's file descriptor table 4. calls kvm_get_kvm() to turn the borrowed reference to the VM into a real reference
The ownership transfer in step 3 must not happen before the reference to the VM becomes a proper, non-borrowed reference, which only happens in step 4. After step 3, an attacker can close the file descriptor and drop the borrowed reference, which can cause the refcount of the kvm object to drop to zero.
This means that we need to grab a reference for the device before anon_inode_getfd(), otherwise the VM can disappear from under us.
Fixes: 852b6d57dc7f ("kvm: add device control API") Cc: stable@kernel.org Signed-off-by: Jann Horn jannh@google.com --- virt/kvm/kvm_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index e4be695eb789..fce48d11ae07 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -2711,14 +2711,15 @@ static int kvm_ioctl_create_device(struct kvm *kvm, return ret; }
+ kvm_get_kvm(kvm); ret = anon_inode_getfd(ops->name, &kvm_device_fops, dev, O_RDWR | O_CLOEXEC); if (ret < 0) { + kvm_put_kvm(kvm); ops->destroy(dev); return ret; }
list_add(&dev->vm_node, &kvm->devices); - kvm_get_kvm(kvm); cd->fd = ret; return 0; }
On Mon, Feb 18, 2019 at 05:53:28PM +0100, Jann Horn wrote:
commit cfa39381173d5f969daf43582c95ad679189cbc9 upstream.
kvm_ioctl_create_device() does the following:
- creates a device that holds a reference to the VM object (with a borrowed
reference, the VM's refcount has not been bumped yet) 2. initializes the device 3. transfers the reference to the device to the caller's file descriptor table 4. calls kvm_get_kvm() to turn the borrowed reference to the VM into a real reference
The ownership transfer in step 3 must not happen before the reference to the VM becomes a proper, non-borrowed reference, which only happens in step 4. After step 3, an attacker can close the file descriptor and drop the borrowed reference, which can cause the refcount of the kvm object to drop to zero.
This means that we need to grab a reference for the device before anon_inode_getfd(), otherwise the VM can disappear from under us.
Fixes: 852b6d57dc7f ("kvm: add device control API") Cc: stable@kernel.org Signed-off-by: Jann Horn jannh@google.com
Hi Jann,
You've dropped Paolo's S-O-B line, was it on purpose?
-- Thanks, Sasha
On Thu, Feb 21, 2019 at 5:10 AM Sasha Levin sashal@kernel.org wrote:
On Mon, Feb 18, 2019 at 05:53:28PM +0100, Jann Horn wrote:
commit cfa39381173d5f969daf43582c95ad679189cbc9 upstream.
kvm_ioctl_create_device() does the following:
- creates a device that holds a reference to the VM object (with a borrowed
reference, the VM's refcount has not been bumped yet) 2. initializes the device 3. transfers the reference to the device to the caller's file descriptor table 4. calls kvm_get_kvm() to turn the borrowed reference to the VM into a real reference
The ownership transfer in step 3 must not happen before the reference to the VM becomes a proper, non-borrowed reference, which only happens in step 4. After step 3, an attacker can close the file descriptor and drop the borrowed reference, which can cause the refcount of the kvm object to drop to zero.
This means that we need to grab a reference for the device before anon_inode_getfd(), otherwise the VM can disappear from under us.
Fixes: 852b6d57dc7f ("kvm: add device control API") Cc: stable@kernel.org Signed-off-by: Jann Horn jannh@google.com
Hi Jann,
You've dropped Paolo's S-O-B line, was it on purpose?
The stable kernel rules don't explicitly say that I'm allowed to keep other people's sign-off lines when manually submitting backports, so I removed it to be on the safe side.
On 21/02/19 14:03, Jann Horn wrote:
On Thu, Feb 21, 2019 at 5:10 AM Sasha Levin sashal@kernel.org wrote:
Hi Jann,
You've dropped Paolo's S-O-B line, was it on purpose?
The stable kernel rules don't explicitly say that I'm allowed to keep other people's sign-off lines when manually submitting backports, so I removed it to be on the safe side.
Yeah, since I wasn't really involved in the creation of this patch, and I had not modified Jann's original submission in any way, I guess it's okay either way.
Paolo
On Mon, Feb 18, 2019 at 05:53:28PM +0100, Jann Horn wrote:
commit cfa39381173d5f969daf43582c95ad679189cbc9 upstream.
kvm_ioctl_create_device() does the following:
- creates a device that holds a reference to the VM object (with a borrowed reference, the VM's refcount has not been bumped yet)
- initializes the device
- transfers the reference to the device to the caller's file descriptor table
- calls kvm_get_kvm() to turn the borrowed reference to the VM into a real reference
The ownership transfer in step 3 must not happen before the reference to the VM becomes a proper, non-borrowed reference, which only happens in step 4. After step 3, an attacker can close the file descriptor and drop the borrowed reference, which can cause the refcount of the kvm object to drop to zero.
This means that we need to grab a reference for the device before anon_inode_getfd(), otherwise the VM can disappear from under us.
Fixes: 852b6d57dc7f ("kvm: add device control API") Cc: stable@kernel.org Signed-off-by: Jann Horn jannh@google.com
virt/kvm/kvm_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Now queued up, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org