| Message ID | 1427289352-6680-1-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 <pbonzini@redhat.com> > --- > 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 <arei.gonglei@huawei.com>
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);
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. 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 <pbonzini@redhat.com> --- Please review. :) exec.c | 1 - 1 file changed, 1 deletion(-)