Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752434AbdLMDFg (ORCPT ); Tue, 12 Dec 2017 22:05:36 -0500 Received: from mail-ot0-f196.google.com ([74.125.82.196]:43041 "EHLO mail-ot0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751799AbdLMDFd (ORCPT ); Tue, 12 Dec 2017 22:05:33 -0500 X-Google-Smtp-Source: ACJfBovfU8fsoJ8tDxzidMASCvm94BsT8Q/20PwsOF3iQHnDiQRIzf9qleqJdnCbY3hgbl2J4HnK3gwUWN0zmni9t3I= MIME-Version: 1.0 In-Reply-To: References: <1513069078-63339-1-git-send-email-wanpeng.li@hotmail.com> From: Wanpeng Li Date: Wed, 13 Dec 2017 11:05:31 +0800 Message-ID: Subject: Re: [PATCH RESEND] KVM: X86: Fix stack-out-of-bounds read in write_mmio To: Paolo Bonzini Cc: "linux-kernel@vger.kernel.org" , kvm , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Wanpeng Li 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-Length: 1227 Lines: 32 2017-12-13 0:07 GMT+08:00 Paolo Bonzini : > On 12/12/2017 09:57, Wanpeng Li wrote: >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index bc5d853..51e7932 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -4690,7 +4690,10 @@ static int write_emulate(struct kvm_vcpu *vcpu, gpa_t gpa, >> >> static int write_mmio(struct kvm_vcpu *vcpu, gpa_t gpa, int bytes, void *val) >> { >> - trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, bytes, gpa, *(u64 *)val); >> + u64 data = 0; >> + >> + memcpy(&data, val, min(8, bytes)); >> + trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, bytes, gpa, data); >> return vcpu_mmio_write(vcpu, gpa, bytes, val); >> } >> >> > > Please do the memcpy in TRACE_EVENT(kvm_mmio)'s TP_fast_assign block. > That is done only when the trace event is active. I still can observe the stack out-of-bounds read warning if keep *(u64 *)val as the parameter since it has already been dereferenced. So maybe we should change the parameter of trace_kvm_mmio() to void *val, however, I'm not sure whether it will break the tracepoint ABI which this article https://lwn.net/Articles/734039/ "Workload analysis with tracing" section pointed out. Regards, Wanpeng Li