From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51375) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adYlx-0000AJ-Sd for qemu-devel@nongnu.org; Wed, 09 Mar 2016 02:43:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1adYls-0002vx-Qm for qemu-devel@nongnu.org; Wed, 09 Mar 2016 02:43:09 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58930) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adYls-0002vO-L0 for qemu-devel@nongnu.org; Wed, 09 Mar 2016 02:43:04 -0500 Date: Wed, 9 Mar 2016 15:42:59 +0800 From: Fam Zheng Message-ID: <20160309074259.GG17947@ad.usersys.redhat.com> References: <1457504091-31887-1-git-send-email-peterx@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1457504091-31887-1-git-send-email-peterx@redhat.com> Subject: Re: [Qemu-devel] [PATCH] hw/i386: fix unbounded stack for load_multiboot List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu Cc: peter.maydell@linaro.org, ehabkost@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, pbonzini@redhat.com, rth@twiddle.net On Wed, 03/09 14:14, Peter Xu wrote: > Use heap rather than stack for kcmdline. > > Signed-off-by: Peter Xu > --- > hw/i386/multiboot.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c > index 9e164e6..bc45394 100644 > --- a/hw/i386/multiboot.c > +++ b/hw/i386/multiboot.c > @@ -324,10 +324,9 @@ int load_multiboot(FWCfgState *fw_cfg, > } > > /* Commandline support */ > - char kcmdline[strlen(kernel_filename) + strlen(kernel_cmdline) + 2]; > - snprintf(kcmdline, sizeof(kcmdline), "%s %s", > - kernel_filename, kernel_cmdline); > + char *kcmdline = g_strdup_printf("%s %s", kernel_filename, kernel_cmdline); > stl_p(bootinfo + MBI_CMDLINE, mb_add_cmdline(&mbs, kcmdline)); > + g_free(kcmdline); > > stl_p(bootinfo + MBI_BOOTLOADER, mb_add_bootloader(&mbs, bootloader_name)); While at it, it's better to move the variable declaration to the beginning of function. Fam