From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751544AbdK3UkO (ORCPT ); Thu, 30 Nov 2017 15:40:14 -0500 Received: from mail-vk0-f68.google.com ([209.85.213.68]:45226 "EHLO mail-vk0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751415AbdK3UkM (ORCPT ); Thu, 30 Nov 2017 15:40:12 -0500 X-Google-Smtp-Source: AGs4zMa1BRZNs22t8WLh+f2sE54G+GKRykDUChz3uA/Zec5efHGiwPDerY2gYOaM7BOP+8mhBvY5CHel/QjyCnthpug= MIME-Version: 1.0 In-Reply-To: <20171026134547.23664-3-pbonzini@redhat.com> References: <20171026134547.23664-1-pbonzini@redhat.com> <20171026134547.23664-3-pbonzini@redhat.com> From: Kees Cook Date: Thu, 30 Nov 2017 12:40:10 -0800 X-Google-Sender-Auth: JTCNspyFW-Ja4NYkmH9OCF8SUVE Message-ID: Subject: Re: [PATCH 2/2] kvm: x86: fix KVM_XEN_HVM_CONFIG ioctl To: Paolo Bonzini Cc: LKML , KVM , Christoffer Dall , Marc Zyngier , Christian Borntraeger , Cornelia Huck , James Hogan , Paul Mackerras , kernel-hardening@lists.openwall.com, =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id vAUKeH3f025195 On Thu, Oct 26, 2017 at 6:45 AM, Paolo Bonzini wrote: > This ioctl is obsolete (it was used by Xenner as far as I know) but > still let's not break it gratuitously... Its handler is copying > directly into struct kvm. Go through a bounce buffer instead, with > the added benefit that we can actually do something useful with the > flags argument---the previous code was exiting with -EINVAL but still > doing the copy. > > This technically is a userspace ABI breakage, but since no one should be > using the ioctl, it's a good occasion to see if someone actually > complains. > > Cc: kernel-hardening@lists.openwall.com > Cc: Kees Cook > Cc: Radim Krčmář > Signed-off-by: Paolo Bonzini > --- > arch/x86/kvm/x86.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 272320eb328c..f32fbfb833b3 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4187,13 +4187,14 @@ long kvm_arch_vm_ioctl(struct file *filp, > mutex_unlock(&kvm->lock); > break; > case KVM_XEN_HVM_CONFIG: { > + struct kvm_xen_hvm_config xhc; > r = -EFAULT; > - if (copy_from_user(&kvm->arch.xen_hvm_config, argp, > - sizeof(struct kvm_xen_hvm_config))) > + if (copy_from_user(&xhc, argp, sizeof(xhc))) > goto out; > r = -EINVAL; > - if (kvm->arch.xen_hvm_config.flags) > + if (xhc.flags) > goto out; > + memcpy(&kvm->arch.xen_hvm_config, &xhc, sizeof(xhc)); > r = 0; > break; > } > -- > 2.14.2 > Hi Paolo, Since this didn't make it via my usercopy tree, do you want to take it via KVM? It is a stand-alone fix, AIUI. Thanks, -Kees -- Kees Cook Pixel Security From mboxrd@z Thu Jan 1 00:00:00 1970 MIME-Version: 1.0 Sender: keescook@google.com In-Reply-To: <20171026134547.23664-3-pbonzini@redhat.com> References: <20171026134547.23664-1-pbonzini@redhat.com> <20171026134547.23664-3-pbonzini@redhat.com> From: Kees Cook Date: Thu, 30 Nov 2017 12:40:10 -0800 Message-ID: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: [kernel-hardening] Re: [PATCH 2/2] kvm: x86: fix KVM_XEN_HVM_CONFIG ioctl To: Paolo Bonzini Cc: LKML , KVM , Christoffer Dall , Marc Zyngier , Christian Borntraeger , Cornelia Huck , James Hogan , Paul Mackerras , kernel-hardening@lists.openwall.com, =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= List-ID: On Thu, Oct 26, 2017 at 6:45 AM, Paolo Bonzini wrote: > This ioctl is obsolete (it was used by Xenner as far as I know) but > still let's not break it gratuitously... Its handler is copying > directly into struct kvm. Go through a bounce buffer instead, with > the added benefit that we can actually do something useful with the > flags argument---the previous code was exiting with -EINVAL but still > doing the copy. > > This technically is a userspace ABI breakage, but since no one should be > using the ioctl, it's a good occasion to see if someone actually > complains. > > Cc: kernel-hardening@lists.openwall.com > Cc: Kees Cook > Cc: Radim Kr=C4=8Dm=C3=A1=C5=99 > Signed-off-by: Paolo Bonzini > --- > arch/x86/kvm/x86.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 272320eb328c..f32fbfb833b3 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4187,13 +4187,14 @@ long kvm_arch_vm_ioctl(struct file *filp, > mutex_unlock(&kvm->lock); > break; > case KVM_XEN_HVM_CONFIG: { > + struct kvm_xen_hvm_config xhc; > r =3D -EFAULT; > - if (copy_from_user(&kvm->arch.xen_hvm_config, argp, > - sizeof(struct kvm_xen_hvm_config))) > + if (copy_from_user(&xhc, argp, sizeof(xhc))) > goto out; > r =3D -EINVAL; > - if (kvm->arch.xen_hvm_config.flags) > + if (xhc.flags) > goto out; > + memcpy(&kvm->arch.xen_hvm_config, &xhc, sizeof(xhc)); > r =3D 0; > break; > } > -- > 2.14.2 > Hi Paolo, Since this didn't make it via my usercopy tree, do you want to take it via KVM? It is a stand-alone fix, AIUI. Thanks, -Kees --=20 Kees Cook Pixel Security