From patchwork Sat Nov 27 20:06:38 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 73297 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 2448BB6EDF for ; Sun, 28 Nov 2010 07:16:24 +1100 (EST) Received: from localhost ([127.0.0.1]:47478 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PMRC1-0003cz-9p for incoming@patchwork.ozlabs.org; Sat, 27 Nov 2010 15:16:21 -0500 Received: from [140.186.70.92] (port=49178 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PMRBX-0003co-CS for qemu-devel@nongnu.org; Sat, 27 Nov 2010 15:15:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PMRBV-0006hn-T5 for qemu-devel@nongnu.org; Sat, 27 Nov 2010 15:15:51 -0500 Received: from adelie.canonical.com ([91.189.90.139]:44893) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PMRBV-0006hb-JU for qemu-devel@nongnu.org; Sat, 27 Nov 2010 15:15:49 -0500 Received: from loganberry.canonical.com ([91.189.90.37]) by adelie.canonical.com with esmtp (Exim 4.69 #1 (Debian)) id 1PMRBU-0004uq-GE for ; Sat, 27 Nov 2010 20:15:48 +0000 Received: from loganberry.canonical.com (localhost [127.0.0.1]) by loganberry.canonical.com (Postfix) with ESMTP id 73E182E8056 for ; Sat, 27 Nov 2010 20:15:48 +0000 (UTC) MIME-Version: 1.0 Date: Sat, 27 Nov 2010 20:06:38 -0000 From: Peter Maydell To: qemu-devel@nongnu.org X-Launchpad-Bug: product=qemu; status=New; importance=Undecided; assignee=None; X-Launchpad-Bug-Private: no X-Launchpad-Bug-Security-Vulnerability: no X-Launchpad-Bug-Commenters: ferringb pmaydell stephen-clarke X-Launchpad-Bug-Reporter: Stephen Clarke (stephen-clarke) X-Launchpad-Bug-Modifier: Peter Maydell (pmaydell) References: <20101007125628.8009.23815.malonedeb@soybean.canonical.com> <20101127192930.5365.13234.malone@potassium.ubuntu.com> Message-Id: Subject: Re: [Qemu-devel] [Bug 656285] Re: arm-semi mishandling SYS_HEAPINFO X-Launchpad-Message-Rationale: Subscriber (QEMU) @qemu-devel-ml Precedence: bulk X-Generated-By: Launchpad (canonical.com); Revision="None"; Instance="initZopeless config overlay" X-Launchpad-Hash: 1b785421c1a7d387824ae626fea48489d3b8dc14 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Reply-To: Bug 656285 <656285@bugs.launchpad.net> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 27 November 2010 19:29, Brian Harring wrote: > Look through linux-user/syscall.c; looks like the flaw is more in do_brk > itself.  Invocations of do_brk *appear* to all assume that it's > basically brk like in it's behaviour- -1 on failure, else a non-negative > value of what the size now is. I think this is wrong. The primary user of do_brk() is the Linux user system call emulation, so do_brk()'s semantics follow those of the kernel implemented syscall. (This is different from those of the brk() library routine, hence the confusion.) That means it returns the old brk value on failure, not -1. [This is documented in the NOTES section of the Linux brk(2) manpage, or you can look at the kernel sources.] You could argue that this is a bit unhelpful for platform-independent code like the ARM semihosting routines which end up coding to an API which might be platform-dependent between linux-user and some-other-user modes... -- PMM --- arm-semi.c.orig 2010-09-21 13:19:15.000000000 +0100 +++ arm-semi.c 2010-10-07 13:23:13.000000000 +0100 @@ -475,7 +475,7 @@ /* Try a big heap, and reduce the size if that fails. */ for (;;) { ret = do_brk(limit); - if (ret != -1) + if (ret == limit) break; limit = (ts->heap_base >> 1) + (limit >> 1); }