Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752882AbdGSIPm (ORCPT ); Wed, 19 Jul 2017 04:15:42 -0400 Received: from atrey.karlin.mff.cuni.cz ([195.113.26.193]:56837 "EHLO atrey.karlin.mff.cuni.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751448AbdGSIPi (ORCPT ); Wed, 19 Jul 2017 04:15:38 -0400 Date: Wed, 19 Jul 2017 10:15:36 +0200 From: Pavel Machek To: Rask Ingemann Lambertsen Cc: Russell King , Richard Genoud , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, nico@linaro.org, gregory.clement@free-electrons.com, Linus Torvalds Subject: Re: [v4.13 regression] ARM: zImage: Fix stack overflow in merge_fdt_bootargs() Message-ID: <20170719081536.GA17727@amd> References: <257f11e3b0010a3a44c28b34a048278f7b960f3b.1500238579.git.rask@formelder.dk> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="mP3DRpeJDSE+ciuQ" Content-Disposition: inline In-Reply-To: <257f11e3b0010a3a44c28b34a048278f7b960f3b.1500238579.git.rask@formelder.dk> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4666 Lines: 155 --mP3DRpeJDSE+ciuQ Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi! > This function is called very early on from head.S and currently sets up a > stack frame of more than 1024 bytes: >=20 > atags_to_fdt.c: In function =E2=80=98merge_fdt_bootargs=E2=80=99: > atags_to_fdt.c:98:1: warning: the frame size of 1032 bytes is larger than= 1024 bytes [-Wframe-larger-than=3D] >=20 > 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 of 4.1.0, > gcc version 5.4.1 20161019 (Debian 5.4.1-3) and tegra20-trimslice.dtb. >=20 > Signed-off-by: Rask Ingemann Lambertsen > Fixes: d0f34a11ddab ("ARM: 7437/1: zImage: Allow DTB command line concate= nation with ATAG_CMDLINE") I tested that it does not break boot on N900. I hoped that it would fix boot on N950 (but no luck there so far). Tested-by: Pavel Machek AFAICT this is regression in v4.13-rc1. Thus it is quite surprising that there are no comments here. Genoud? Russell? Pavel >=20 > 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. >=20 > arch/arm/boot/compressed/atags_to_fdt.c | 59 +++++++++++++++++++--------= ------ > 1 file changed, 35 insertions(+), 24 deletions(-) >=20 > diff --git a/arch/arm/boot/compressed/atags_to_fdt.c b/arch/arm/boot/comp= ressed/atags_to_fdt.c > index 9448aa0c6686..ede23ef2e889 100644 > --- a/arch/arm/boot/compressed/atags_to_fdt.c > +++ b/arch/arm/boot/compressed/atags_to_fdt.c > @@ -55,6 +55,27 @@ static const void *getprop(const void *fdt, const char= *node_path, > return fdt_getprop(fdt, offset, property, len); > } > =20 > +static void *getprop_w(void *fdt, const char *node_path, > + const char *property, int *len) > +{ > + int offset =3D fdt_path_offset(fdt, node_path); > + > + if (offset =3D=3D -FDT_ERR_NOTFOUND) > + return NULL; > + > + return fdt_getprop_w(fdt, offset, property, len); > +} > + > +static int appendprop_string(void *fdt, const char *node_path, > + const char *property, const char *string) > +{ > + int offset =3D node_offset(fdt, node_path); > + > + if (offset < 0) > + return offset; > + return fdt_appendprop_string(fdt, offset, property, string); > +} > + > static uint32_t get_cell_size(const void *fdt) > { > int len; > @@ -66,35 +87,25 @@ static uint32_t get_cell_size(const void *fdt) > return cell_size; > } > =20 > -static void merge_fdt_bootargs(void *fdt, const char *fdt_cmdline) > -{ > - char cmdline[COMMAND_LINE_SIZE]; > - const char *fdt_bootargs; > - char *ptr =3D cmdline; > - int len =3D 0; > - > - /* copy the fdt command line into the buffer */ > - fdt_bootargs =3D 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 +=3D len - 1; > - } > - > - /* and append the ATAG_CMDLINE */ > - if (fdt_cmdline) { > - len =3D strlen(fdt_cmdline); > - if (ptr - cmdline + len + 2 < COMMAND_LINE_SIZE) { > - *ptr++ =3D ' '; > - memcpy(ptr, fdt_cmdline, len); > - ptr +=3D len; > - } > - } > - *ptr =3D '\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) > +{ > + char *fdt_bootargs; > + int len =3D 0; > + > + /* With no ATAG command line or an empty one, there is nothing to do. */ > + if (!atag_cmdline || strlen(atag_cmdline) =3D=3D 0) > + return; > + > + fdt_bootargs =3D getprop_w(fdt, "/chosen", "bootargs", &len); > + > + /* With no FDT command line or an empty one, just use the ATAG one. */ > + if (!fdt_bootargs || len <=3D 1) { > + setprop_string(fdt, "/chosen", "bootargs", atag_cmdline); > + return; > + } > + fdt_bootargs[len - 1] =3D ' '; > + appendprop_string(fdt, "/chosen", "bootargs", atag_cmdline); > } > =20 > /* --=20 (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blo= g.html --mP3DRpeJDSE+ciuQ Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iEYEARECAAYFAllvFSgACgkQMOfwapXb+vJ1NACgn60RWotuQiu9+vkg1zZeBsCu TTYAn1urjxWFNrB7LtsfxSYUdERNTNpN =pgco -----END PGP SIGNATURE----- --mP3DRpeJDSE+ciuQ--