Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753418AbdGSIrf (ORCPT ); Wed, 19 Jul 2017 04:47:35 -0400 Received: from bhuna.collabora.co.uk ([46.235.227.227]:39959 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752799AbdGSIrc (ORCPT ); Wed, 19 Jul 2017 04:47:32 -0400 Date: Wed, 19 Jul 2017 10:47:27 +0200 From: Sebastian Reichel To: Pavel Machek Cc: Rask Ingemann Lambertsen , 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: <20170719084727.lhjkr4kccc2xjw7r@earth> References: <257f11e3b0010a3a44c28b34a048278f7b960f3b.1500238579.git.rask@formelder.dk> <20170719081536.GA17727@amd> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="psh4v5fpyjz52m75" Content-Disposition: inline In-Reply-To: <20170719081536.GA17727@amd> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5906 Lines: 178 --psh4v5fpyjz52m75 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Wed, Jul 19, 2017 at 10:15:36AM +0200, Pavel Machek wrote: > > 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 th= an 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 > >=20 > > Signed-off-by: Rask Ingemann Lambertsen > > Fixes: d0f34a11ddab ("ARM: 7437/1: zImage: Allow DTB command line conca= tenation with ATAG_CMDLINE") >=20 > 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). >=20 > Tested-by: Pavel Machek >=20 > AFAICT this is regression in v4.13-rc1. Thus it is quite surprising > that there are no comments here. No, it's not. I definitely saw the warning before (and see it now) and images can be booted on all of the phones I'm working on (i.e. N900, N950, Droid 4). Anyawys thanks for fixing this properly. I will add the patch to my dev branch and give it a test during the day. -- Sebastian >=20 > Genoud? Russell? >=20 > Pavel >=20 >=20 > >=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/co= mpressed/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 ch= ar *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 > --=20 > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/b= log.html --psh4v5fpyjz52m75 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEE72YNB0Y/i3JqeVQT2O7X88g7+poFAllvHJwACgkQ2O7X88g7 +pq65Q//cNvnGj/plmQ8vc4LGeS/bi1g/N4kCB7Z0uRvL9CmQ7itmfROVfgoKjbq zEI5IeHOjqJ7xrquPNtEjfZKkpSUf16MVMipY/qlVwuQMtrvdm7WszPXcSdgD19W OzgxbOXAspcRVVN+H6+naJBVM223y5mXr4kfgzIx50n1Yxk3pl1wNacaYJUVvm3L Fhhm2pF2T8Z6ExfyjigDHVuDcVwv3NRviI3FFHKGf6bqlO4xpWmjsozwqYabNlR0 RUQ09ik+k0hvP0a72FNCoK6rPR+uxXnKWyX/DpNF/tKmWPuf2F7fcVatIsiJKSw/ OGWCtD8/pucJaY8FFIyKh+cCINIfaHJzI7sP2VTuOCp1/ahT16qyiPeKqg/16EHl rLf6pz1e+yOJ30tiRKLRI49vh+cTNGPdTcY6iP2j5By330Ct9c/KmsxCROPJ47jr 5wJNum4Q83bchWJFnDETPzdQX0JW3KOmflQnyGuyjHKo6mlJrJ/Au3/9CVvdWhQ0 BMSevBqGtMH05Qlks7euXD26UADE1rJ9m1DDx0Hp4jeLd/nBbwZJTrErl5eTb8MM 9bqPqFHb3YVO9ba7IvEfTWPLxEmmXVPRnu1TKpOWg8n0djCdiPezKRcpMo6LxQNm 7kH1KX57kOfHzAlUMfjsHSOdsi8b7ex65NrsW8kRkjKFJIh19Ts= =4MN7 -----END PGP SIGNATURE----- --psh4v5fpyjz52m75--