From patchwork Tue Jun 25 08:49:39 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 254062 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id D1F8E2C0040 for ; Tue, 25 Jun 2013 18:50:06 +1000 (EST) Received: from localhost ([::1]:49408 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UrOwu-000413-V8 for incoming@patchwork.ozlabs.org; Tue, 25 Jun 2013 04:50:04 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49090) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UrOwe-000405-IV for qemu-devel@nongnu.org; Tue, 25 Jun 2013 04:49:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UrOwd-0006Cw-7T for qemu-devel@nongnu.org; Tue, 25 Jun 2013 04:49:48 -0400 Received: from mail-qc0-x233.google.com ([2607:f8b0:400d:c01::233]:52334) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UrOwd-0006Cm-2j for qemu-devel@nongnu.org; Tue, 25 Jun 2013 04:49:47 -0400 Received: by mail-qc0-f179.google.com with SMTP id e11so6978429qcx.24 for ; Tue, 25 Jun 2013 01:49:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:x-enigmail-version:content-type :content-transfer-encoding; bh=+2zAmw4q8NSO5VZm5acb+vuucLlBvbnvE+dmG66QQxg=; b=p1Fktrj1YUOYNJvQxrK2md20Gqs6B8woDR+DzZN5Q3VfvXT+9QbqL2JcSL1ni4A16G SAVAyC8bpUG0vD2KEbPtCpdc7axN8hTTabO1PGwH7+W7C/cIaTu7oXIK6L2Hv8FHehdI Vg/Vu3E8zMo3mHZTroVhMug+DdFAeXMPHsefGjSfwXnd4R7ts7q1Lih9cV7LYnrj0Ja1 ShiA/p6Y8cAfrR8HLuAfaVltSBQHM8UEkJuZd7aFHmGG0Cw1cZ544CUK14t5KcDsPb5q V/iGL3QmN9tYALCPdN/CCjLzclO6Yo7Dw3zp/V7kp9lumOijERqCB4APYagldYpRQBxa idxw== X-Received: by 10.224.177.135 with SMTP id bi7mr31265115qab.56.1372150186439; Tue, 25 Jun 2013 01:49:46 -0700 (PDT) Received: from yakj.usersys.redhat.com (net-37-116-217-184.cust.dsl.vodafone.it. [37.116.217.184]) by mx.google.com with ESMTPSA id 11sm24622904qek.1.2013.06.25.01.49.43 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Tue, 25 Jun 2013 01:49:45 -0700 (PDT) Message-ID: <51C959A3.9030504@redhat.com> Date: Tue, 25 Jun 2013 10:49:39 +0200 From: Paolo Bonzini User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 MIME-Version: 1.0 To: Alexey Kardashevskiy References: <1370348041-6768-1-git-send-email-pbonzini@redhat.com> <1370348041-6768-4-git-send-email-pbonzini@redhat.com> <51B96676.9010707@ozlabs.ru> <51B98A8A.5020200@ozlabs.ru> <51BAEBC4.4030607@ozlabs.ru> In-Reply-To: <51BAEBC4.4030607@ozlabs.ru> X-Enigmail-Version: 1.5.1 X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2607:f8b0:400d:c01::233 Cc: peter.maydell@linaro.org, Paul Mackerras , qemu-devel@nongnu.org, Avi Kivity Subject: Re: [Qemu-devel] [PATCH v2 03/17] memory: add ref/unref calls X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Il 14/06/2013 12:09, Alexey Kardashevskiy ha scritto: > Hi. > > Ok. Back to the bug with this patch. The initial problem with this patch is > that "make check" fails. > > Please help with subpages. > > It turned out that tests use MALLOC_PERTURB_ which is normally off. Who > does not know - this is a way to tell glibc to fill released memory with > some value and then debug accesses to released memory. Some bright mind > made it random what confuses a lot (and btw valgrind found nothing :-/ ). > So I spend some time before figured out how to reproduce it outside of the > qtest thingy. > > The tree is qemu.org/master "bd5c51e Michael Roth qemu-char: don't issue > CHR_EVENT_OPEN in a BH" + replayed patches till the one from $subj on top > of it. QEMU is configured as "configure --target-list=x86_64-softmmu". > > The magic is: > > export MALLOC_PERTURB_=123 > nc -l -U ~/qtest-16318.sock & > nc -l -U ~/qtest-16318.qmp & > x86_64-softmmu/qemu-system-x86_64 \ > -qtest unix:/home/alexey/qtest-16318.sock,nowait \ > -qtest-log /dev/null \ > -qmp unix:/home/alexey/qtest-16318.qmp,nowait \ > -pidfile ~/qtest-16318.pid -machine accel=qtest -vnc none > > Immediate crash at (the very last backtrace in this mail is that crash). It's a use-after-free, strange that valgrind found nothing. The subpage is freed in destroy_page_desc before being used in phys_sections_clear. The fix is to move freeing the phys_sections in phys_sections_clear, which is also cleaner. diff --git a/exec.c b/exec.c index fa5de88..93907e9 100644 --- a/exec.c +++ b/exec.c @@ -762,17 +762,6 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env, static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end, uint16_t section); static subpage_t *subpage_init(AddressSpace *as, hwaddr base); -static void destroy_page_desc(uint16_t section_index) -{ - MemoryRegionSection *section = &phys_sections[section_index]; - MemoryRegion *mr = section->mr; - - if (mr->subpage) { - subpage_t *subpage = container_of(mr, subpage_t, iomem); - memory_region_destroy(&subpage->iomem); - g_free(subpage); - } -} static void destroy_l2_mapping(PhysPageEntry *lp, unsigned level) { @@ -787,8 +776,6 @@ static void destroy_l2_mapping(PhysPageEntry *lp, unsigned level) for (i = 0; i < L2_SIZE; ++i) { if (!p[i].is_leaf) { destroy_l2_mapping(&p[i], level - 1); - } else { - destroy_page_desc(p[i].ptr); } } lp->is_leaf = 0; @@ -819,11 +806,20 @@ static uint16_t phys_section_add(MemoryRegionSection *section) return phys_sections_nb++; } +static void phys_section_destroy(MemoryRegion *mr) +{ + memory_region_unref(mr); + + if (mr->subpage) { + subpage_t *subpage = container_of(mr, subpage_t, iomem); + memory_region_destroy(&subpage->iomem); + g_free(subpage); + } +} + static void phys_sections_clear(void) { while (phys_sections_nb > 0) { MemoryRegionSection *section = &phys_sections[--phys_sections_nb]; - memory_region_unref(section->mr); + phys_section_destroy(section->mr); } }