From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39023) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaxVb-0001gE-12 for qemu-devel@nongnu.org; Wed, 25 Mar 2015 22:26:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YaxVX-0003Gq-Ps for qemu-devel@nongnu.org; Wed, 25 Mar 2015 22:26:58 -0400 Received: from szxga02-in.huawei.com ([119.145.14.65]:17292) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaxVX-0003D7-0w for qemu-devel@nongnu.org; Wed, 25 Mar 2015 22:26:55 -0400 Message-ID: <55136E55.9000703@huawei.com> Date: Thu, 26 Mar 2015 10:26:29 +0800 From: Gonglei MIME-Version: 1.0 References: <1427289352-6680-1-git-send-email-pbonzini@redhat.com> In-Reply-To: <1427289352-6680-1-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] exec: avoid possible overwriting of mmaped area in qemu_ram_remap List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org On 2015/3/25 21:15, Paolo Bonzini wrote: > It is not necessary to munmap an area before remapping it with MAP_FIXED; > if the memory region specified by addr and len overlaps pages of any > existing mapping, then the overlapped part of the existing mapping will > be discarded. > Yes, it is. > On the other hand, if QEMU does munmap the pages, there is a small > probability that another mmap sneaks in and catches the just-freed > portion of the address space. In effect, munmap followed by > mmap(MAP_FIXED) is a use-after-free error, and Coverity flags it > as such. Fix it. > > Signed-off-by: Paolo Bonzini > --- > Please review. :) > > exec.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/exec.c b/exec.c > index 8b922db..6d1e1e4 100644 > --- a/exec.c > +++ b/exec.c > @@ -1638,7 +1638,6 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length) > abort(); > } else { > flags = MAP_FIXED; > - munmap(vaddr, length); > if (block->fd >= 0) { > flags |= (block->flags & RAM_SHARED ? > MAP_SHARED : MAP_PRIVATE); > Looks good to me, so Reviewed-by: Gonglei