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 Cc: kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org To: Marc Zyngier Return-path: Received: from mail-wm0-f42.google.com ([74.125.82.42]:37529 "EHLO mail-wm0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751103AbcBXLjq (ORCPT ); Wed, 24 Feb 2016 06:39:46 -0500 Received: by mail-wm0-f42.google.com with SMTP id g62so25716310wme.0 for ; Wed, 24 Feb 2016 03:39:45 -0800 (PST) Content-Disposition: inline In-Reply-To: <1455723260-23793-1-git-send-email-marc.zyngier@arm.com> Sender: kvm-owner@vger.kernel.org List-ID: 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 >