All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
@ 2012-06-25 12:42 Tetsuyuki Kobayashi
  2012-06-27 17:40 ` Tom Rini
  0 siblings, 1 reply; 16+ messages in thread
From: Tetsuyuki Kobayashi @ 2012-06-25 12:42 UTC (permalink / raw)
  To: u-boot

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>
---
 arch/arm/cpu/armv7/cpu.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c
index c6fa8ef..6104cb2 100644
--- a/arch/arm/cpu/armv7/cpu.c
+++ b/arch/arm/cpu/armv7/cpu.c
@@ -37,8 +37,11 @@
 #include <asm/cache.h>
 #include <asm/armv7.h>
 
+__attribute__((naked)) /* don't save anything to stack even if compiled with -O0 */
 void save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3)
 {
+	/* stack is not yet initialized */
+	asm("bx lr");
 }
 
 void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3)
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
  2012-06-25 12:42 [U-Boot] [PATCH] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0 Tetsuyuki Kobayashi
@ 2012-06-27 17:40 ` Tom Rini
  2012-06-28  1:14   ` Tetsuyuki Kobayashi
  0 siblings, 1 reply; 16+ messages in thread
From: Tom Rini @ 2012-06-27 17:40 UTC (permalink / raw)
  To: u-boot

On Mon, Jun 25, 2012 at 09:42:29PM +0900, 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>
> ---
>  arch/arm/cpu/armv7/cpu.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c
> index c6fa8ef..6104cb2 100644
> --- a/arch/arm/cpu/armv7/cpu.c
> +++ b/arch/arm/cpu/armv7/cpu.c
> @@ -37,8 +37,11 @@
>  #include <asm/cache.h>
>  #include <asm/armv7.h>
>  
> +__attribute__((naked)) /* don't save anything to stack even if compiled with -O0 */
>  void save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3)
>  {
> +	/* stack is not yet initialized */
> +	asm("bx lr");
>  }

Please add <linux/compiler.h> and use __naked instead.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120627/33caee2b/attachment.pgp>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
  2012-06-27 17:40 ` Tom Rini
@ 2012-06-28  1:14   ` Tetsuyuki Kobayashi
  2012-06-28 11:35     ` [U-Boot] [PATCH v2] " Tetsuyuki Kobayashi
  0 siblings, 1 reply; 16+ messages in thread
From: Tetsuyuki Kobayashi @ 2012-06-28  1:14 UTC (permalink / raw)
  To: u-boot

Hi Tom, thank you for reviewing.

(2012/06/28 2:40), Tom Rini wrote:
> On Mon, Jun 25, 2012 at 09:42:29PM +0900, 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>
>> ---
>>   arch/arm/cpu/armv7/cpu.c |    3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c
>> index c6fa8ef..6104cb2 100644
>> --- a/arch/arm/cpu/armv7/cpu.c
>> +++ b/arch/arm/cpu/armv7/cpu.c
>> @@ -37,8 +37,11 @@
>>   #include<asm/cache.h>
>>   #include<asm/armv7.h>
>>
>> +__attribute__((naked)) /* don't save anything to stack even if compiled with -O0 */
>>   void save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3)
>>   {
>> +	/* stack is not yet initialized */
>> +	asm("bx lr");
>>   }
>
> Please add<linux/compiler.h>  and use __naked instead.  Thanks!
>
OK. I will post V2 patch.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
  2012-06-28  1:14   ` Tetsuyuki Kobayashi
@ 2012-06-28 11:35     ` Tetsuyuki Kobayashi
  2012-06-28 14:57       ` Tom Rini
  0 siblings, 1 reply; 16+ messages in thread
From: Tetsuyuki Kobayashi @ 2012-06-28 11:35 UTC (permalink / raw)
  To: u-boot

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))


 arch/arm/cpu/armv7/cpu.c |    4 ++++
 1 file changed, 4 insertions(+)

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
  2012-06-28 11:35     ` [U-Boot] [PATCH v2] " Tetsuyuki Kobayashi
@ 2012-06-28 14:57       ` Tom Rini
  2012-06-28 16:00         ` koba
  2012-06-29  9:36         ` [U-Boot] [PATCH v3] " Tetsuyuki Kobayashi
  0 siblings, 2 replies; 16+ messages in thread
From: Tom Rini @ 2012-06-28 14:57 UTC (permalink / raw)
  To: u-boot

On 06/28/2012 04:35 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>
> ---
> Changes for v2:
>  - include <linux/compiler.h> and use __naked instead of __attribute__((naked))
> 
> 
>  arch/arm/cpu/armv7/cpu.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c
> index c6fa8ef..3e2a75c 100644
> --- a/arch/arm/cpu/armv7/cpu.c
> +++ b/arch/arm/cpu/armv7/cpu.c
> @@ -36,9 +36,13 @@
>  #include <asm/system.h>
>  #include <asm/cache.h>
>  #include <asm/armv7.h>
> +#include <linux/compiler.h>
>  
> +__naked /* don't save anything to stack even if compiled with -O0 */
>  void save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3)

The usual form (here and kernel) is:
void __naked save_boot_params_default(...)

Same for __weak and so on.  Thanks!

-- 
Tom

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
  2012-06-28 14:57       ` Tom Rini
@ 2012-06-28 16:00         ` koba
  2012-06-29  9:36         ` [U-Boot] [PATCH v3] " Tetsuyuki Kobayashi
  1 sibling, 0 replies; 16+ messages in thread
From: koba @ 2012-06-28 16:00 UTC (permalink / raw)
  To: u-boot


On 2012/06/28, at 23:57, Tom Rini wrote:

> On 06/28/2012 04:35 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>
>> ---
>> Changes for v2:
>> - include <linux/compiler.h> and use __naked instead of __attribute__((naked))
>> 
>> 
>> arch/arm/cpu/armv7/cpu.c |    4 ++++
>> 1 file changed, 4 insertions(+)
>> 
>> diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c
>> index c6fa8ef..3e2a75c 100644
>> --- a/arch/arm/cpu/armv7/cpu.c
>> +++ b/arch/arm/cpu/armv7/cpu.c
>> @@ -36,9 +36,13 @@
>> #include <asm/system.h>
>> #include <asm/cache.h>
>> #include <asm/armv7.h>
>> +#include <linux/compiler.h>
>> 
>> +__naked /* don't save anything to stack even if compiled with -O0 */
>> void save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3)
> 
> The usual form (here and kernel) is:
> void __naked save_boot_params_default(...)
> 
> Same for __weak and so on.  Thanks!
> 
Oh, I should grep __naked before posting this.
I will try V3.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v3] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
  2012-06-28 14:57       ` Tom Rini
  2012-06-28 16:00         ` koba
@ 2012-06-29  9:36         ` Tetsuyuki Kobayashi
  2012-06-29 14:21           ` Tom Rini
  2012-07-05 11:57           ` Albert ARIBAUD
  1 sibling, 2 replies; 16+ messages in thread
From: Tetsuyuki Kobayashi @ 2012-06-29  9:36 UTC (permalink / raw)
  To: u-boot

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(-)

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)
-- 1.7.9.5 

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v3] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
  2012-06-29  9:36         ` [U-Boot] [PATCH v3] " Tetsuyuki Kobayashi
@ 2012-06-29 14:21           ` Tom Rini
  2012-07-05  9:25             ` Albert ARIBAUD
  2012-07-05 11:57           ` Albert ARIBAUD
  1 sibling, 1 reply; 16+ messages in thread
From: Tom Rini @ 2012-06-29 14:21 UTC (permalink / raw)
  To: u-boot

-----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-----

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v3] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
  2012-06-29 14:21           ` Tom Rini
@ 2012-07-05  9:25             ` Albert ARIBAUD
  0 siblings, 0 replies; 16+ messages in thread
From: Albert ARIBAUD @ 2012-07-05  9:25 UTC (permalink / raw)
  To: u-boot

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v3] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
  2012-06-29  9:36         ` [U-Boot] [PATCH v3] " Tetsuyuki Kobayashi
  2012-06-29 14:21           ` Tom Rini
@ 2012-07-05 11:57           ` Albert ARIBAUD
  2012-07-05 16:18             ` Tom Rini
  1 sibling, 1 reply; 16+ messages in thread
From: Albert ARIBAUD @ 2012-07-05 11:57 UTC (permalink / raw)
  To: u-boot

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,
-- 
Albert.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v3] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
  2012-07-05 11:57           ` Albert ARIBAUD
@ 2012-07-05 16:18             ` Tom Rini
  2012-07-05 17:10               ` Albert ARIBAUD
  0 siblings, 1 reply; 16+ messages in thread
From: Tom Rini @ 2012-07-05 16:18 UTC (permalink / raw)
  To: u-boot

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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120705/9dbd121d/attachment.pgp>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v3] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
  2012-07-05 16:18             ` Tom Rini
@ 2012-07-05 17:10               ` Albert ARIBAUD
  2012-07-06  6:10                 ` [U-Boot] [PATCH v4] " Tetsuyuki Kobayashi
  0 siblings, 1 reply; 16+ messages in thread
From: Albert ARIBAUD @ 2012-07-05 17:10 UTC (permalink / raw)
  To: u-boot

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,
-- 
Albert.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v4] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
  2012-07-05 17:10               ` Albert ARIBAUD
@ 2012-07-06  6:10                 ` Tetsuyuki Kobayashi
  2012-07-06 14:00                   ` Tom Rini
  0 siblings, 1 reply; 16+ messages in thread
From: Tetsuyuki Kobayashi @ 2012-07-06  6:10 UTC (permalink / raw)
  To: u-boot

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>
---
Hi Tom, Albert,

I rewrite them in asm language and put it to start.S.
No warning now.
I tested it quickly on my kzm9g board.

Changes for v2:
 - include <linux/compiler.h> and use __naked instead of __attribute__((naked))

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

Changes for v4:
 - v3 causes following warnings
  cpu.c: In function 'save_boot_params_default':
  cpu.c:48:1: warning: -fstack-usage not supported for this target [enabled by default]
 - move save_boot_params_default() and save_boot_params() from cpu.c to start.S
   and write them in asm language

 arch/arm/cpu/armv7/cpu.c   |    7 -------
 arch/arm/cpu/armv7/start.S |   15 +++++++++++++++
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c
index c6fa8ef..b0677f4 100644
--- a/arch/arm/cpu/armv7/cpu.c
+++ b/arch/arm/cpu/armv7/cpu.c
@@ -37,13 +37,6 @@
 #include <asm/cache.h>
 #include <asm/armv7.h>
 
-void save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3)
-{
-}
-
-void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3)
-	__attribute__((weak, alias("save_boot_params_default")));
-
 int cleanup_before_linux(void)
 {
 	/*
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index 261835b..4feade5 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -350,6 +350,21 @@ ENTRY(cpu_init_crit)
 ENDPROC(cpu_init_crit)
 #endif
 
+/*************************************************************************
+ *
+ * void 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
+ *	
+ *************************************************************************/
+ENTRY(save_boot_params_default)
+	bx	lr			@ back to my caller
+ENDPROC(save_boot_params_default)
+
+	.weak	save_boot_params
+	.set	save_boot_params, save_boot_params_default
+
 #ifndef CONFIG_SPL_BUILD
 /*
  *************************************************************************
--
1.7.9.5 

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v4] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
  2012-07-06  6:10                 ` [U-Boot] [PATCH v4] " Tetsuyuki Kobayashi
@ 2012-07-06 14:00                   ` Tom Rini
  2012-07-07  7:14                     ` [U-Boot] [PATCH v5] " Tetsuyuki Kobayashi
  0 siblings, 1 reply; 16+ messages in thread
From: Tom Rini @ 2012-07-06 14:00 UTC (permalink / raw)
  To: u-boot

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/05/2012 11:10 PM, 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> --- Hi Tom, 
> Albert,
> 
> I rewrite them in asm language and put it to start.S. No warning 
> now. I tested it quickly on my kzm9g board.
> 
> Changes for v2: - include <linux/compiler.h> and use __naked 
> instead of __attribute__((naked))
> 
> Changes for v3: - move __naked after void - reformat comments
> 
> Changes for v4: - v3 causes following warnings cpu.c: In function 
> 'save_boot_params_default': cpu.c:48:1: warning: -fstack-usage not 
> supported for this target [enabled by default] - move 
> save_boot_params_default() and save_boot_params() from cpu.c to 
> start.S and write them in asm language
> 
> arch/arm/cpu/armv7/cpu.c   |    7 ------- 
> arch/arm/cpu/armv7/start.S |   15 +++++++++++++++ 2 files changed, 
> 15 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c 
> index c6fa8ef..b0677f4 100644 --- a/arch/arm/cpu/armv7/cpu.c +++ 
> b/arch/arm/cpu/armv7/cpu.c @@ -37,13 +37,6 @@ #include 
> <asm/cache.h> #include <asm/armv7.h>
> 
> -void save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3) -{ 
> -} - -void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3) - 
> __attribute__((weak, alias("save_boot_params_default"))); - int 
> cleanup_before_linux(void) { /* diff --git 
> a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 
> 261835b..4feade5 100644 --- a/arch/arm/cpu/armv7/start.S +++ 
> b/arch/arm/cpu/armv7/start.S @@ -350,6 +350,21 @@ 
> ENTRY(cpu_init_crit) ENDPROC(cpu_init_crit) #endif
> 
> +/*************************************************************************
>
>
> 
+ *
> + * void 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 + * + 
> *************************************************************************/
>
>
> 
+ENTRY(save_boot_params_default)
> +	bx	lr			@ back to my caller +ENDPROC(save_boot_params_default) +
>  +	.weak	save_boot_params +	.set	save_boot_params, 
> save_boot_params_default + #ifndef CONFIG_SPL_BUILD /* 
> *************************************************************************

We
>
> 
shouldn't, I believe, need to call this save_boot_params_default
and then alias it.  We should be able to just call it save_boot_params
and declare it weak, then omap3 for example will override a link time
(this should be easily verifiable with objdump).  Thanks!

- -- 
Tom

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

iQIcBAEBAgAGBQJP9u9rAAoJENk4IS6UOR1W0oAQAJnM2ENHS3yhv3i+bUTJpXjI
sFPiQAd9t/QzDpCr7/xm05+mTciRUmzJ9tIF/XUQbVZPJZvxnfit43EVxaNuyQyZ
RC6Le9RBHzSFXmx2EwCUrNzog7Q3oOjAS6GVa2UaGsLf2lTcJrhomYNtk30cSwUw
sSQobL/jr5668Obuf1WQxEbextTt6JLTeWqCfsEBesg1eENqkskWQHg6zA8HD85i
RLYux9VEhZ/PaunH5K1RNOG3SzXZzhbxTwl4EpQ+6V8ocGqRG2tghWW/o5oG2Mjv
/i65FD0RbQhzqjX4z2far4PYBvVs+LIwPIUKZmP7A3QDrzoapCC7bk2nRBsfLPMX
xN3NeU13KsBre+7IFvMYvCCrZYSSMYRqAjNFxZwEJMeH7p3zGcDPGdAKaqCEDCJU
chNzcfhFiYezGGT4x7Sk5Apw7dj7kReC4mcuXAr9KHDV0tZy78nqqOa8wgxKgsz1
S4fiiDRwIHY+U/KgLW8ayv8VzSnDLTN6GJ23MWQiv1BXHdq77F2vDpytepR9m6BU
JT3Dtesdh5GmA0PSaRq9Wfh0TzTPkYJd91XD8JXulCpRJmxa70O5dVrE6mXWDyAQ
98esMJxyyOEFCcsRb7U2/NsRHLbuLjAKdF3nyzGaBp1qhvY/d5zsYwgNfIx3dpWk
SZohRX5nOA0Y+ZcphGih
=51Xu
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v5] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
  2012-07-06 14:00                   ` Tom Rini
@ 2012-07-07  7:14                     ` Tetsuyuki Kobayashi
  2012-07-09  8:55                       ` Tom Rini
  0 siblings, 1 reply; 16+ messages in thread
From: Tetsuyuki Kobayashi @ 2012-07-07  7:14 UTC (permalink / raw)
  To: u-boot

save_boot_params_default() in cpu.c accesses uninitialized stack area
when it compiled with -O0 (not optimized).
This patch removes save_boot_params_default() and put the equivalent in start.S

Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
---
Hi Tom, Albert,

I rewrite it again.
I tested it quickly on my kzm9g board, and also build it for omap4_panda and
checked the generated code by objdump command.

Changes for v2:
 - include <linux/compiler.h> and use __naked instead of __attribute__((naked))

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

Changes for v4:
 - v3 causes following warnings
  cpu.c: In function 'save_boot_params_default':
  cpu.c:48:1: warning: -fstack-usage not supported for this target [enabled by default]
 - move save_boot_params_default() and save_boot_params() from cpu.c to start.S
   and write them in asm language

Changes for v5
 - rename save_boot_parames_default() to save_boot_params() and drop aliasing
 - move the code after relocate_code (nearer to callee)
 - modify commit log

 arch/arm/cpu/armv7/cpu.c   |    7 -------
 arch/arm/cpu/armv7/start.S |   14 ++++++++++++++
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c
index c6fa8ef..b0677f4 100644
--- a/arch/arm/cpu/armv7/cpu.c
+++ b/arch/arm/cpu/armv7/cpu.c
@@ -37,13 +37,6 @@
 #include <asm/cache.h>
 #include <asm/armv7.h>
 
-void save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3)
-{
-}
-
-void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3)
-	__attribute__((weak, alias("save_boot_params_default")));
-
 int cleanup_before_linux(void)
 {
 	/*
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index 261835b..bf734fb 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -293,6 +293,20 @@ ENDPROC(relocate_code)
 
 /*************************************************************************
  *
+ * void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3)
+ *	__attribute__((weak));
+ *
+ * Stack pointer is not yet initialized at this moment
+ * Don't save anything to stack even if compiled with -O0
+ *
+ *************************************************************************/
+ENTRY(save_boot_params)
+	bx	lr			@ back to my caller
+ENDPROC(save_boot_params)
+	.weak	save_boot_params
+
+/*************************************************************************
+ *
  * cpu_init_cp15
  *
  * Setup CP15 registers (cache, MMU, TLBs). The I-cache is turned on unless
-- 
1.7.9.5 

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v5] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0
  2012-07-07  7:14                     ` [U-Boot] [PATCH v5] " Tetsuyuki Kobayashi
@ 2012-07-09  8:55                       ` Tom Rini
  0 siblings, 0 replies; 16+ messages in thread
From: Tom Rini @ 2012-07-09  8:55 UTC (permalink / raw)
  To: u-boot

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/07/2012 12:14 AM, Tetsuyuki Kobayashi wrote:
> save_boot_params_default() in cpu.c accesses uninitialized stack
> area when it compiled with -O0 (not optimized). This patch removes
> save_boot_params_default() and put the equivalent in start.S
> 
> 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/

iQIcBAEBAgAGBQJP+pyHAAoJENk4IS6UOR1Ww6sP/iFYvofEDFPBjHEZtfOFqmHU
+D49nDI24nlA68WlD12s3muteEAS3fsyxs74wHKbBpz0vIph0twCM41i78khzBoJ
Rore4b380dGwU3RJD4pZ++RELrTyrlTVLo3aQg0rp7K+KaCms5YByftBqwhIeI3r
fw1zIO2mFCY5uz3PPxjwO8++vcaH/Fk1G6MNRaz4RgnG/dZx6tQVEtuQbTmyltrM
mJiBypZU7FCHxEnBbfBkJerrTxEbM/chZQEy3QFMYpfs3JEx4QqFwSN6i8YV3yKQ
AmeB+BbAfU+tm8xxTfnXKZOCP50i70Tir+42XV4PJ6PY9qfOriDaVl9V1bP/8FK3
rZodkAqc5V0ieRbeBIYouJhrPKMQ1b3uZ8FsSV3uU7HvWx4Rjy/yP5T0dW/j52cy
+TFd1ZH/B2wUU1ihSfV2r80v9+tQ59KYeBUjN0i2pdnAdk/ELbyREKakYM1vWyUN
g4UaR6YTtx+SJH48nbgzoLgyYeyWCZKJrNbJRSQ7CckcluoHHeTbTnY3bYAbntJm
jMwdBeBUjZcxz123ca1lFY5elJiPu2mCDbNDv4J1pw1bqfEc7WY1xm73hZ6YIr1X
JmyyIOyXhOPDad3A3Th93bwbLbtV38GqXMspJ4aXkT+j9IW/RdK9tKWtQZ5cIcE5
abwLAFb/jiGpxmSuEo3g
=6jT2
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2012-07-09  8:55 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-25 12:42 [U-Boot] [PATCH] arm: bugfix: save_boot_params_default accesses uninitalized stack when -O0 Tetsuyuki Kobayashi
2012-06-27 17:40 ` Tom Rini
2012-06-28  1:14   ` Tetsuyuki Kobayashi
2012-06-28 11:35     ` [U-Boot] [PATCH v2] " Tetsuyuki Kobayashi
2012-06-28 14:57       ` Tom Rini
2012-06-28 16:00         ` koba
2012-06-29  9:36         ` [U-Boot] [PATCH v3] " Tetsuyuki Kobayashi
2012-06-29 14:21           ` Tom Rini
2012-07-05  9:25             ` Albert ARIBAUD
2012-07-05 11:57           ` Albert ARIBAUD
2012-07-05 16:18             ` Tom Rini
2012-07-05 17:10               ` Albert ARIBAUD
2012-07-06  6:10                 ` [U-Boot] [PATCH v4] " Tetsuyuki Kobayashi
2012-07-06 14:00                   ` Tom Rini
2012-07-07  7:14                     ` [U-Boot] [PATCH v5] " Tetsuyuki Kobayashi
2012-07-09  8:55                       ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.