From patchwork Mon Jun 20 16:20:12 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Riku Voipio X-Patchwork-Id: 101155 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 9DD97B6F73 for ; Tue, 21 Jun 2011 02:48:00 +1000 (EST) Received: from localhost ([::1]:47744 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QYhdk-0001PH-JU for incoming@patchwork.ozlabs.org; Mon, 20 Jun 2011 12:47:56 -0400 Received: from eggs.gnu.org ([140.186.70.92]:58509) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QYhDS-0003Ue-7r for qemu-devel@nongnu.org; Mon, 20 Jun 2011 12:20:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QYhDQ-0008Mn-Iq for qemu-devel@nongnu.org; Mon, 20 Jun 2011 12:20:45 -0400 Received: from afflict.kos.to ([92.243.29.197]:34624) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QYhDP-0008Md-W3 for qemu-devel@nongnu.org; Mon, 20 Jun 2011 12:20:44 -0400 Received: from kos.to (a88-115-163-181.elisa-laajakaista.fi [88.115.163.181]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by afflict.kos.to (Postfix) with ESMTPSA id 2D3C226689; Mon, 20 Jun 2011 16:20:42 +0000 (UTC) Received: by kos.to (sSMTP sendmail emulation); Mon, 20 Jun 2011 19:20:41 +0300 From: riku.voipio@iki.fi To: qemu-devel@nongnu.org Date: Mon, 20 Jun 2011 19:20:12 +0300 Message-Id: X-Mailer: git-send-email 1.7.4.1 In-Reply-To: References: MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 92.243.29.197 Cc: =?UTF-8?q?C=C3=A9dric=20VINCENT?= Subject: [Qemu-devel] [PATCH 07/18] linux-user: Fix the computation of the requested heap size 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 From: Cédric VINCENT There were two remaining bugs in the previous implementation of do_brk(): 1. the value of "new_alloc_size" was one page too large when the requested brk was aligned on a host page boundary. 2. no new pages should be (re-)allocated when the requested brk is in the range of the pages that were already allocated previsouly (for the same purpose). Technically these pages are never unmapped in the current implementation. The problem/fix can be reproduced/validated with the following test case: #include /* syscall(2), */ #include /* SYS_brk, */ #include /* puts(3), */ #include /* exit(3), EXIT_*, */ int main() { int current_brk = 0; int new_brk; int failure = 0; void test(int increment) { static int test_number = 0; test_number++; new_brk = syscall(SYS_brk, current_brk + increment); if (new_brk == current_brk) { printf("test %d fails\n", test_number); failure++; } current_brk = new_brk; } /* Initialization. */ test(0); /* Does QEMU overlap host pages? */ test(HOST_PAGE_SIZE); test(HOST_PAGE_SIZE); /* Does QEMU allocate the same host page twice? */ test(-HOST_PAGE_SIZE); test(HOST_PAGE_SIZE); if (!failure) { printf("success\n"); exit(EXIT_SUCCESS); } else { exit(EXIT_FAILURE); } } Signed-off-by: Cédric VINCENT Reviewed-by: Christophe Guillon Signed-off-by: Riku Voipio --- linux-user/syscall.c | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index b975730..be27f53 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -709,16 +709,17 @@ char *target_strerror(int err) static abi_ulong target_brk; static abi_ulong target_original_brk; +static abi_ulong brk_page; void target_set_brk(abi_ulong new_brk) { target_original_brk = target_brk = HOST_PAGE_ALIGN(new_brk); + brk_page = HOST_PAGE_ALIGN(target_brk); } /* do_brk() must return target values and target errnos. */ abi_long do_brk(abi_ulong new_brk) { - abi_ulong brk_page; abi_long mapped_addr; int new_alloc_size; @@ -727,9 +728,8 @@ abi_long do_brk(abi_ulong new_brk) if (new_brk < target_original_brk) return target_brk; - brk_page = HOST_PAGE_ALIGN(target_brk); - - /* If the new brk is less than this, set it and we're done... */ + /* If the new brk is less than the highest page reserved to the + * target heap allocation, set it and we're done... */ if (new_brk < brk_page) { target_brk = new_brk; return target_brk; @@ -741,13 +741,14 @@ abi_long do_brk(abi_ulong new_brk) * itself); instead we treat "mapped but at wrong address" as * a failure and unmap again. */ - new_alloc_size = HOST_PAGE_ALIGN(new_brk - brk_page + 1); + new_alloc_size = HOST_PAGE_ALIGN(new_brk - brk_page); mapped_addr = get_errno(target_mmap(brk_page, new_alloc_size, PROT_READ|PROT_WRITE, MAP_ANON|MAP_PRIVATE, 0, 0)); if (mapped_addr == brk_page) { target_brk = new_brk; + brk_page = HOST_PAGE_ALIGN(target_brk); return target_brk; } else if (mapped_addr != -1) { /* Mapped but at wrong address, meaning there wasn't actually