From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42194) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adUeL-00006a-Rk for qemu-devel@nongnu.org; Tue, 08 Mar 2016 22:19:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1adUeL-0001P0-3J for qemu-devel@nongnu.org; Tue, 08 Mar 2016 22:19:01 -0500 Date: Wed, 9 Mar 2016 11:18:44 +0800 From: Peter Xu Message-ID: <20160309031844.GH2377@pxdev.xzpeter.org> References: <1457420446-25276-1-git-send-email-peterx@redhat.com> <1457420446-25276-3-git-send-email-peterx@redhat.com> <87twkhe6bm.fsf@blackfin.pond.sub.org> <56DEC2BF.5040502@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <56DEC2BF.5040502@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/8] block: fix unbounded stack for dump_qdict List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Kevin Wolf , Markus Armbruster , qemu-block@nongnu.org, qemu-devel@nongnu.org On Tue, Mar 08, 2016 at 01:17:03PM +0100, Paolo Bonzini wrote: > > > On 08/03/2016 09:12, Markus Armbruster wrote: > > I'm afraid this isn't a good idea. It relies on the non-local argument > > that nobody will ever put a key longer than 255 into a qdict that gets > > dumped. That may even be the case, but you need to *prove* it, not just > > assert it. The weakest acceptable proof might be assertions in every > > place that put keys into a dict that might get dumped. I suspect that's > > practical and maintainable only if there's a single place that does it. > > > > If this was a good idea, I'd recommend to avoid the awkward macro: > > > > char key[256]; > > int i; > > > > assert(strlen(entry->key) + 1 <= ARRAY_SIZE(key)); > > > > There are several other ways to limit the stack usage: > > > > 1. Move the array from stack to heap. Fine unless it's on a hot path. > > As far as I can tell, this dumping business is for HMP and qemu-io, > > i.e. not hot. > > I think this is the best. You can just g_strdup, modify in place, print > and free. g_strdup() will bring one more loop? One to copy the strings, one for replacing "-" to " ". Though I will first need to replace g_malloc0() with g_malloc(), which seems more suitable here. :) Thanks! Peter