From patchwork Mon Mar 13 03:02:01 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Xu X-Patchwork-Id: 737958 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)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3vhN2l2sq5z9s7K for ; Mon, 13 Mar 2017 14:02:39 +1100 (AEDT) Received: from localhost ([::1]:49859 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cnGFo-0003Yo-VS for incoming@patchwork.ozlabs.org; Sun, 12 Mar 2017 23:02:36 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51586) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cnGFQ-0003Yi-IY for qemu-devel@nongnu.org; Sun, 12 Mar 2017 23:02:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cnGFM-0005tz-LZ for qemu-devel@nongnu.org; Sun, 12 Mar 2017 23:02:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52078) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cnGFM-0005tL-Cy for qemu-devel@nongnu.org; Sun, 12 Mar 2017 23:02:08 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A8259C049D5B; Mon, 13 Mar 2017 03:02:07 +0000 (UTC) Received: from pxdev.xzpeter.org (ovpn-8-40.pek2.redhat.com [10.72.8.40]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v2D322Sm015976 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Sun, 12 Mar 2017 23:02:05 -0400 Date: Mon, 13 Mar 2017 11:02:01 +0800 From: Peter Xu To: "Michael S. Tsirkin" Message-ID: <20170313030201.GH17299@pxdev.xzpeter.org> References: <1489345956-29167-1-git-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1489345956-29167-1-git-send-email-mst@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Mon, 13 Mar 2017 03:02:07 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: Re: [Qemu-devel] [PATCH] memory: use 128 bit in info mtree X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paolo Bonzini , Mark Cave-Ayland , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" On Sun, Mar 12, 2017 at 09:12:43PM +0200, Michael S. Tsirkin wrote: > info mtree is doing 64 bit math to figure out > addresses from offsets, this does not work ncorrectly > incase of overflow. > > Overflow usually indicates a guest bug, so this is unusual > but reporting correct addresses makes it easier to discover > what is going on. A tiny issue would be that we will always dump 128 bits even if nothing went wrong. IMHO That's slightly awkward. Not sure whether that will confuse people since they should be thinking why we need that on 64bit systems... Do you like below one instead? It'll keep the old interface, but just warn user explicity when something wrong happens, and it's much easier and obvious imho (along with a tiny cleanup): (the code is not tested even for compile) ---------8<----------- Thanks, -- peterx diff --git a/memory.c b/memory.c index 284894b..64b0a60 100644 --- a/memory.c +++ b/memory.c @@ -2494,6 +2494,7 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, MemoryRegionListHead submr_print_queue; const MemoryRegion *submr; unsigned int i; + hwaddr cur_start, cur_end; if (!mr) { return; @@ -2503,6 +2504,18 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, mon_printf(f, MTREE_INDENT); } + cur_start = base + mr->addr; + cur_end = cur_start + MR_SIZE(mr->size); + + /* + * Try to detect overflow of memory ranges. This should never + * happen normally. When it happens, we dump something to warn the + * user who is observing this. + */ + if (cur_start < base || cur_end < cur_start) { + mon_printf(f, "[DETECTED OVERFLOW!] "); + } + if (mr->alias) { MemoryRegionList *ml; bool found = false; @@ -2522,8 +2535,7 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): alias %s @%s " TARGET_FMT_plx "-" TARGET_FMT_plx "%s\n", - base + mr->addr, - base + mr->addr + MR_SIZE(mr->size), + cur_start, cur_end, mr->priority, memory_region_type((MemoryRegion *)mr), memory_region_name(mr), @@ -2534,8 +2546,7 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, } else { mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): %s%s\n", - base + mr->addr, - base + mr->addr + MR_SIZE(mr->size), + cur_start, cur_end, mr->priority, memory_region_type((MemoryRegion *)mr), memory_region_name(mr), @@ -2562,7 +2573,7 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, } QTAILQ_FOREACH(ml, &submr_print_queue, queue) { - mtree_print_mr(mon_printf, f, ml->mr, level + 1, base + mr->addr, + mtree_print_mr(mon_printf, f, ml->mr, level + 1, cur_start, alias_print_queue); } --------->8-----------