diff mbox

[U-Boot,v3] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0

Message ID 4FED7715.2060709@kmckk.co.jp
State Accepted
Commit fa042186b932e9b9ee9a2fd8a04a3acf7c70d224
Delegated to: Albert ARIBAUD
Headers show

Commit Message

Tetsuyuki Kobayashi June 29, 2012, 9:36 a.m. UTC
save_boot_params_default() in cpu.c accesses uninitialized stack area
when it compiled with -O0 (not optimized).

Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
---
Changes for v2:
 - include <linux/compiler.h> and use __naked instead of __attribute__((naked))

Changes for v3:
 - move __naked after void
 - reformat comments

 arch/arm/cpu/armv7/cpu.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

-- 1.7.9.5

Comments

Tom Rini June 29, 2012, 2:21 p.m. UTC | #1
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/29/2012 02:36 AM, Tetsuyuki Kobayashi wrote:
> save_boot_params_default() in cpu.c accesses uninitialized stack
> area when it compiled with -O0 (not optimized).
> 
> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>

Acked-by: Tom Rini <trini@ti.com>

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJP7boFAAoJENk4IS6UOR1WX18P/i+/SO3IcUVfLfuKHbc60W10
bZO7amiudkVDngroXJUl8jDZ6ersCsawOA+WxkuEOS0SjcwQsU8bfM0XbR7TiQMG
XJDpD4/woLzEFjkrSqpw0BTsYadAFJWhLVKIx60+4LTUdcL/JNktaCtsKuXWJI21
yG4lWmzECBGmT1JT3AXT3KtQ0WKKlWVxtPpluw8qniEYnUdiXmKSBC42OqRfJWRc
ZJjn7ZLNqszf+C3W4H170vwLcpEEjw6KBi2joSaaaEJrJT/ll6jkZEMP0ZUPxGPo
+aJeUVwoUWFsnCuPfIxQ6vNZjDm/slJaTh+B7ZMQ05cNlFYBt/WwSDfJrCnzIgda
rHxs8DbNU4z3WCCjIkvFk87WMIR2aNXv0ojpmy7thliP/nJNfDHwbuTZuvQlL+1l
keeMfIuGQpeWU+qRPrB92bb+vOmaO3tgKVmXGeJ6eT8h5uDJpMagVdYfe6wVb+Cl
ukgQctTWm4L846acHwhOIq2r+GQCTZpVktNgANnZHufMZ2p2uxnRMklZYswJW8Ko
+EigMMe3dcw6BgWfwkPK20FdzGT3OBSVuTwbxw6lCJVzITzPQmB/GY2lxUzDmURi
x9ssawTWgIAap+OBM7oR9+TOaPRMtkbqWFziClWbwmVgjAxI3k8VFzN+mk1nEcYO
KkdqOLTSccjOgKSZH5pn
=lr2Y
-----END PGP SIGNATURE-----
Albert ARIBAUD July 5, 2012, 9:25 a.m. UTC | #2
Hi Tetsuyuki, Tom,

On Fri, 29 Jun 2012 07:21:57 -0700, Tom Rini <trini@ti.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 06/29/2012 02:36 AM, Tetsuyuki Kobayashi wrote:
> > save_boot_params_default() in cpu.c accesses uninitialized stack
> > area when it compiled with -O0 (not optimized).
> > 
> > Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
> 
> Acked-by: Tom Rini <trini@ti.com>

I'll take it in as soon as marvell and atmel are pulled in.
 
Amicalement,
Albert ARIBAUD July 5, 2012, 11:57 a.m. UTC | #3
Hi Tetsuyuki,

On Fri, 29 Jun 2012 18:36:21 +0900, Tetsuyuki Kobayashi
<koba@kmckk.co.jp> wrote:
> save_boot_params_default() in cpu.c accesses uninitialized stack area
> when it compiled with -O0 (not optimized).
> 
> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
> ---
> Changes for v2:
>  - include <linux/compiler.h> and use __naked instead of
> __attribute__((naked))
> 
> Changes for v3:
>  - move __naked after void
>  - reformat comments
> 
>  arch/arm/cpu/armv7/cpu.c |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

Applied to u-boot-arm/master, thanks.

Amicalement,
Tom Rini July 5, 2012, 4:18 p.m. UTC | #4
On Thu, Jul 05, 2012 at 01:57:26PM +0200, Albert ARIBAUD wrote:
> Hi Tetsuyuki,
> 
> On Fri, 29 Jun 2012 18:36:21 +0900, Tetsuyuki Kobayashi
> <koba@kmckk.co.jp> wrote:
> > save_boot_params_default() in cpu.c accesses uninitialized stack area
> > when it compiled with -O0 (not optimized).
> > 
> > Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
> > ---
> > Changes for v2:
> >  - include <linux/compiler.h> and use __naked instead of
> > __attribute__((naked))
> > 
> > Changes for v3:
> >  - move __naked after void
> >  - reformat comments
> > 
> >  arch/arm/cpu/armv7/cpu.c |    8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> Applied to u-boot-arm/master, thanks.

Oh no...
cpu.c: In function 'save_boot_params_default':
cpu.c:48:1: warning: -fstack-usage not supported for this target [enabled by default]

Last time we made a const uint32 out of the instruction instead (see
494931a).  I don't think that's appropriate here however.  Maybe we can
declare the function weak in assembly instead, then we won't need the
naked part and won't have this warning.
Albert ARIBAUD July 5, 2012, 5:10 p.m. UTC | #5
Hi Tom,

On Thu, 5 Jul 2012 09:18:28 -0700, Tom Rini <trini@ti.com> wrote:
> On Thu, Jul 05, 2012 at 01:57:26PM +0200, Albert ARIBAUD wrote:
> > Hi Tetsuyuki,
> > 
> > On Fri, 29 Jun 2012 18:36:21 +0900, Tetsuyuki Kobayashi
> > <koba@kmckk.co.jp> wrote:
> > > save_boot_params_default() in cpu.c accesses uninitialized stack area
> > > when it compiled with -O0 (not optimized).
> > > 
> > > Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
> > > ---
> > > Changes for v2:
> > >  - include <linux/compiler.h> and use __naked instead of
> > > __attribute__((naked))
> > > 
> > > Changes for v3:
> > >  - move __naked after void
> > >  - reformat comments
> > > 
> > >  arch/arm/cpu/armv7/cpu.c |    8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > Applied to u-boot-arm/master, thanks.
> 
> Oh no...
> cpu.c: In function 'save_boot_params_default':
> cpu.c:48:1: warning: -fstack-usage not supported for this target [enabled by default]
> 
> Last time we made a const uint32 out of the instruction instead (see
> 494931a).  I don't think that's appropriate here however.  Maybe we can
> declare the function weak in assembly instead, then we won't need the
> naked part and won't have this warning.

Meanwhile I'll remove this patch from my tree.

Amicalement,
diff mbox

Patch

diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c
index c6fa8ef..9eb484a 100644
--- a/arch/arm/cpu/armv7/cpu.c
+++ b/arch/arm/cpu/armv7/cpu.c
@@ -36,9 +36,15 @@ 
 #include <asm/system.h>
 #include <asm/cache.h>
 #include <asm/armv7.h>
+#include <linux/compiler.h>
 
-void save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3)
+void __naked save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3)
 {
+	/*
+	 * Stack pointer is not yet initialized
+	 * Don't save anything to stack even if compiled with -O0
+	 */
+	asm("bx lr");
 }
 
 void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3)