From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [PATCH] arm/arm64: KVM: Feed initialized memory to MMIO accesses Date: Wed, 24 Feb 2016 12:40:44 +0100 Message-ID: <20160224114044.GA18451@cbox> References: <1455723260-23793-1-git-send-email-marc.zyngier@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1455723260-23793-1-git-send-email-marc.zyngier@arm.com> Sender: kvm-owner@vger.kernel.org To: Marc Zyngier Cc: kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org List-Id: kvmarm@lists.cs.columbia.edu On Wed, Feb 17, 2016 at 03:34:20PM +0000, Marc Zyngier wrote: > On an MMIO access, we always copy the on-stack buffer info > the shared "run" structure, even if this is a read access. > This ends up leaking up to 8 bytes of uninitialized memory > into userspace. I think it only leaks 'len' bytes to userspace ;) > > An obvious fix for this one is to only perform the copy if > this is an actual write. Reviewed-by: Christoffer Dall > > Signed-off-by: Marc Zyngier > --- > arch/arm/kvm/mmio.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c > index 7f33b20..0f6600f 100644 > --- a/arch/arm/kvm/mmio.c > +++ b/arch/arm/kvm/mmio.c > @@ -206,7 +206,8 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run, > run->mmio.is_write = is_write; > run->mmio.phys_addr = fault_ipa; > run->mmio.len = len; > - memcpy(run->mmio.data, data_buf, len); > + if (is_write) > + memcpy(run->mmio.data, data_buf, len); > > if (!ret) { > /* We handled the access successfully in the kernel. */ > -- > 2.1.4 > From mboxrd@z Thu Jan 1 00:00:00 1970 From: christoffer.dall@linaro.org (Christoffer Dall) Date: Wed, 24 Feb 2016 12:40:44 +0100 Subject: [PATCH] arm/arm64: KVM: Feed initialized memory to MMIO accesses In-Reply-To: <1455723260-23793-1-git-send-email-marc.zyngier@arm.com> References: <1455723260-23793-1-git-send-email-marc.zyngier@arm.com> Message-ID: <20160224114044.GA18451@cbox> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, Feb 17, 2016 at 03:34:20PM +0000, Marc Zyngier wrote: > On an MMIO access, we always copy the on-stack buffer info > the shared "run" structure, even if this is a read access. > This ends up leaking up to 8 bytes of uninitialized memory > into userspace. I think it only leaks 'len' bytes to userspace ;) > > An obvious fix for this one is to only perform the copy if > this is an actual write. Reviewed-by: Christoffer Dall > > Signed-off-by: Marc Zyngier > --- > arch/arm/kvm/mmio.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c > index 7f33b20..0f6600f 100644 > --- a/arch/arm/kvm/mmio.c > +++ b/arch/arm/kvm/mmio.c > @@ -206,7 +206,8 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run, > run->mmio.is_write = is_write; > run->mmio.phys_addr = fault_ipa; > run->mmio.len = len; > - memcpy(run->mmio.data, data_buf, len); > + if (is_write) > + memcpy(run->mmio.data, data_buf, len); > > if (!ret) { > /* We handled the access successfully in the kernel. */ > -- > 2.1.4 >