Patchwork [U-Boot] disable security warning flags when possible

login
register
mail settings
Submitter Mike Frysinger
Date April 25, 2011, 6:06 p.m.
Message ID <1303754800-14317-1-git-send-email-vapier@gentoo.org>
Download mbox | patch
Permalink /patch/92767/
State Accepted
Commit 6262e4e74e2cdb9f231dc71c9893d4a4bd1e88df
Delegated to: Wolfgang Denk
Headers show

Comments

Mike Frysinger - April 25, 2011, 6:06 p.m.
Some toolchains enable security warning flags by default, but these don't
really make sense in the u-boot world.  Such as forcing changes like:
	-printf(foo);
	+printf("%s", foo);

So disable the flags when the compiler supports them.  Linux has already
merged a similar change in their build system.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 config.mk |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
Joakim Tjernlund - April 25, 2011, 7:16 p.m.
Mike Frysinger <vapier@gentoo.org> wrote on 2011/04/25 20:06:40:
>
> Some toolchains enable security warning flags by default, but these don't
> really make sense in the u-boot world.  Such as forcing changes like:
>    -printf(foo);
>    +printf("%s", foo);
>
> So disable the flags when the compiler supports them.  Linux has already
> merged a similar change in their build system.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  config.mk |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/config.mk b/config.mk
> index fa46ff1..97d2631 100644
> --- a/config.mk
> +++ b/config.mk
> @@ -191,6 +191,10 @@ CFLAGS := $(CPPFLAGS) -Wall -Wstrict-prototypes
>  endif
>
>  CFLAGS += $(call cc-option,-fno-stack-protector)
> +# Some toolchains enable security related warning flags by default,
> +# but they don't make much sense in the u-boot world, so disable them.
> +CFLAGS += $(call cc-option,-Wno-format-nonliteral)
> +CFLAGS += $(call cc-option,-Wno-format-security)

hmm, now we have this warning in the tree:
   miiphyutil.c: In function 'miiphy_register':
   miiphyutil.c:144: warning: format not a string literal and no format arguments
which is a
	sprintf(new_dev->name, name);

This warning goes away with your patch and I not sure that is a good thing. Keeping just:
   CFLAGS += $(call cc-option,-Wno-format-nonliteral)
makes the warning stay.

 Jocke

PS.
gcc 3.4.6 also complains:
  miiphyutil.c: In function `miiphy_read':
  miiphyutil.c:304: warning: comparison is always false due to limited range of data type
which looks like a real one. Strange gcc 4.4.5 has lost that one.
Mike Frysinger - April 25, 2011, 7:30 p.m.
On Mon, Apr 25, 2011 at 15:16, Joakim Tjernlund wrote:
> Mike Frysinger wrote on 2011/04/25 20:06:40:
>> Some toolchains enable security warning flags by default, but these don't
>> really make sense in the u-boot world.  Such as forcing changes like:
>>    -printf(foo);
>>    +printf("%s", foo);
>>
>> So disable the flags when the compiler supports them.  Linux has already
>> merged a similar change in their build system.
>>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> ---
>>  config.mk |    4 ++++
>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/config.mk b/config.mk
>> index fa46ff1..97d2631 100644
>> --- a/config.mk
>> +++ b/config.mk
>> @@ -191,6 +191,10 @@ CFLAGS := $(CPPFLAGS) -Wall -Wstrict-prototypes
>>  endif
>>
>>  CFLAGS += $(call cc-option,-fno-stack-protector)
>> +# Some toolchains enable security related warning flags by default,
>> +# but they don't make much sense in the u-boot world, so disable them.
>> +CFLAGS += $(call cc-option,-Wno-format-nonliteral)
>> +CFLAGS += $(call cc-option,-Wno-format-security)
>
> hmm, now we have this warning in the tree:
>   miiphyutil.c: In function 'miiphy_register':
>   miiphyutil.c:144: warning: format not a string literal and no format arguments
> which is a
>        sprintf(new_dev->name, name);
>
> This warning goes away with your patch and I not sure that is a good thing. Keeping just:
>   CFLAGS += $(call cc-option,-Wno-format-nonliteral)
> makes the warning stay.

i dont think the existing code is buggy.  certainly changing it to
strcpy(new_dev->name, name) is fine, but again doing
sprintf(new_dev->name, "%s", name) is wrong.

> gcc 3.4.6 also complains:
>  miiphyutil.c: In function `miiphy_read':
>  miiphyutil.c:304: warning: comparison is always false due to limited range of data type
> which looks like a real one. Strange gcc 4.4.5 has lost that one.

this is -Wtype-limits now which makes sense to me ... this shouldnt be
under the "security" umbrella

CFLAGS += $(call cc-option,-Wtype-limits)
-mike
Wolfgang Denk - July 26, 2011, 2:35 p.m.
Dear Mike Frysinger,

In message <1303754800-14317-1-git-send-email-vapier@gentoo.org> you wrote:
> Some toolchains enable security warning flags by default, but these don't
> really make sense in the u-boot world.  Such as forcing changes like:
> 	-printf(foo);
> 	+printf("%s", foo);
> 
> So disable the flags when the compiler supports them.  Linux has already
> merged a similar change in their build system.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  config.mk |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

Patch

diff --git a/config.mk b/config.mk
index fa46ff1..97d2631 100644
--- a/config.mk
+++ b/config.mk
@@ -191,6 +191,10 @@  CFLAGS := $(CPPFLAGS) -Wall -Wstrict-prototypes
 endif
 
 CFLAGS += $(call cc-option,-fno-stack-protector)
+# Some toolchains enable security related warning flags by default,
+# but they don't make much sense in the u-boot world, so disable them.
+CFLAGS += $(call cc-option,-Wno-format-nonliteral)
+CFLAGS += $(call cc-option,-Wno-format-security)
 
 # $(CPPFLAGS) sets -g, which causes gcc to pass a suitable -g<format>
 # option to the assembler.