From patchwork Sat Jul 22 13:02:26 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Rask Ingemann Lambertsen X-Patchwork-Id: 792467 Return-Path: X-Original-To: incoming-imx@patchwork.ozlabs.org Delivered-To: patchwork-incoming-imx@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=lists.infradead.org (client-ip=65.50.211.133; helo=bombadil.infradead.org; envelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="LaDBjoQ1"; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xF79k2P0pz9s7m for ; Sat, 22 Jul 2017 23:03:36 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Date:To:MIME-Version:Subject:From: Message-Id:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=z1/cdIG2xx2KgnrUNQPQewShKHcp3Xfer88r34+h3X8=; b=LaDBjoQ1DldOr/ ogUVBvO+KsuvjebidhLGIidNkpP0gUBfIvJNXXxH/k7U1c3Bs7KCpUW4WPanD7qaEpc38VVPyeKbD 8qaL8IDKzEmgVRS7Ys4G08HEcMatipHIoLBjRJZu/64yavC45OHDixk/kbkVxUl+6aO+Hsh5VLMCh f2QGOQYBgD5k6RdMWP9Y2dAoCKpu969FUqdn2kXAseixV/BTd6SvVttt8rtrWP5l2HpNpP6ph2LjH KzChCBhw9aXtWFJiHsWiUqi0SsqL9aVAJquM+dxiDIJsSBK/ssZiDW7aherFokHT3MjdyGszEM8z3 ggILc3WCRZ2Xhm0BamoQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dYu49-00018G-MZ; Sat, 22 Jul 2017 13:03:29 +0000 Received: from customer-85-204-195-167.ip4.gigabit.dk ([85.204.195.167] helo=customer-2a00-7660-0ca7-0000-0000-0000-0000-0b1b.ip6.gigabit.dk) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dYu43-00017P-W7 for linux-arm-kernel@lists.infradead.org; Sat, 22 Jul 2017 13:03:26 +0000 Received: by customer-2a00-7660-0ca7-0000-0000-0000-0000-0b1b.ip6.gigabit.dk (Postfix, from userid 1000) id 3B397C76; Sat, 22 Jul 2017 15:02:26 +0200 (CEST) Message-Id: From: Rask Ingemann Lambertsen Subject: [PATCH v2] ARM: zImage: Fix stack overflow in merge_fdt_bootargs() MIME-Version: 1.0 To: Russell King Date: Sat, 22 Jul 2017 15:02:26 +0200 (CEST) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170722_060324_426096_622E1832 X-CRM114-Status: GOOD ( 16.80 ) X-Spam-Score: 1.0 (+) X-Spam-Report: SpamAssassin version 3.4.1 on bombadil.infradead.org summary: Content analysis details: (1.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] 1.0 RDNS_DYNAMIC Delivered to internal network by host with dynamic-looking rDNS 2.0 HELO_DYNAMIC_IPADDR Relay HELO'd using suspicious hostname (IP addr 1) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Richard Genoud , Sebastian Reichel , linux-arm-kernel@lists.infradead.org, Pavel Machek , linux-kernel@vger.kernel.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org List-Id: linux-imx-kernel.lists.patchwork.ozlabs.org This function is called very early on from head.S and currently sets up a stack frame of more than 1024 bytes: atags_to_fdt.c: In function ‘merge_fdt_bootargs’: atags_to_fdt.c:98:1: warning: the frame size of 1032 bytes is larger than 1024 bytes [-Wframe-larger-than=] This causes a crash and failure to boot with some combinations of kernel version, gcc version and dtb, such as kernel version 4.1-rc1 or 4.1.0, gcc version 5.4.1 20161019 (Debian 5.4.1-3) and tegra20-trimslice.dtb. With this patch, merge_fdt_bootargs() is rewritten to not use a large buffer, thereby solving the problem of the stack overflow. As a side effect of this rewrite, you no longer get a space added in front of the kernel command line when no bootargs property was found in the FDT. Signed-off-by: Rask Ingemann Lambertsen Tested-by: Pavel Machek Tested-by: Sebastian Reichel Fixes: d0f34a11ddab ("ARM: 7437/1: zImage: Allow DTB command line concatenation with ATAG_CMDLINE") Reviewed-by: Richard Genoud --- I have tested that this works properly when a) only the FDT bootargs are provided, b) only the ATAG command line is provided, and c) both the FDT bootargs and the ATAG command line are provided. Changes from v1 to v2: The original "bootargs" NUL terminator is changed into a space only when appenprop_string() succeeds. Commented by Richard Genoud. arch/arm/boot/compressed/atags_to_fdt.c | 59 +++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/arch/arm/boot/compressed/atags_to_fdt.c b/arch/arm/boot/compressed/atags_to_fdt.c index 9448aa0c6686..87a5fba5a28e 100644 --- a/arch/arm/boot/compressed/atags_to_fdt.c +++ b/arch/arm/boot/compressed/atags_to_fdt.c @@ -55,6 +55,29 @@ static const void *getprop(const void *fdt, const char *node_path, return fdt_getprop(fdt, offset, property, len); } +static int appendprop_string(void *fdt, const char *node_path, + const char *property, const char *string) +{ + int offset = node_offset(fdt, node_path); + + if (offset < 0) + return offset; + return fdt_appendprop_string(fdt, offset, property, string); +} + +static int setprop_inplace_partial(void *fdt, const char *node_path, + const char *property, unsigned int idx, + const void *val, int len) +{ + int offset = node_offset(fdt, node_path); + + if (offset < 0) + return offset; + return fdt_setprop_inplace_namelen_partial(fdt, offset, property, + strlen(property), idx, + val, len); +} + static uint32_t get_cell_size(const void *fdt) { int len; @@ -66,35 +89,27 @@ static uint32_t get_cell_size(const void *fdt) return cell_size; } -static void merge_fdt_bootargs(void *fdt, const char *fdt_cmdline) -{ - char cmdline[COMMAND_LINE_SIZE]; - const char *fdt_bootargs; - char *ptr = cmdline; - int len = 0; - - /* copy the fdt command line into the buffer */ - fdt_bootargs = getprop(fdt, "/chosen", "bootargs", &len); - if (fdt_bootargs) - if (len < COMMAND_LINE_SIZE) { - memcpy(ptr, fdt_bootargs, len); - /* len is the length of the string - * including the NULL terminator */ - ptr += len - 1; - } - - /* and append the ATAG_CMDLINE */ - if (fdt_cmdline) { - len = strlen(fdt_cmdline); - if (ptr - cmdline + len + 2 < COMMAND_LINE_SIZE) { - *ptr++ = ' '; - memcpy(ptr, fdt_cmdline, len); - ptr += len; - } - } - *ptr = '\0'; - - setprop_string(fdt, "/chosen", "bootargs", cmdline); +/* This is called early on from head.S, so it can't use much stack. */ +static void merge_fdt_bootargs(void *fdt, const char *atag_cmdline) +{ + const char *fdt_bootargs; + int len = 0; + + /* With no ATAG command line or an empty one, there is nothing to do. */ + if (!atag_cmdline || strlen(atag_cmdline) == 0) + return; + + fdt_bootargs = getprop(fdt, "/chosen", "bootargs", &len); + + /* With no FDT command line or an empty one, just use the ATAG one. */ + if (!fdt_bootargs || len <= 1) { + setprop_string(fdt, "/chosen", "bootargs", atag_cmdline); + return; + } + if (appendprop_string(fdt, "/chosen", "bootargs", atag_cmdline) < 0) + return; + /* Replace the previous NUL terminator with a space. */ + setprop_inplace_partial(fdt, "/chosen", "bootargs", len - 1, " ", 1); } /*