[PATCH] Kbuild: Enable the gcc flag -Wformat-security (was: Re: [PATCH] Kbuild: Disable the -Wformat-security gcc flag)

[PATCH] Kbuild: Enable the gcc flag -Wformat-security (was: Re: [PATCH] Kbuild: Disable the -Wformat-security gcc flag)

[Floris Kraak]

On Thu, Feb 5, 2009 at 9:26 AM, Floris Kraak <randakar@gmail.com> wrote:
>
> As I noted applying a patch that does the reverse and enables the
> check instead is perfectly acceptable to me. Long term somebody
> probably needs to go through all of them and fix (most of) them
> anyway.
>

Here goes:

[PATCH] Kbuild: Enable the gcc flag -Wformat-security

Some distributions have enabled the gcc flag -Wformat-security by
default. This causes about 150 new warnings to show up in an
allyesconfig kernel build that are never seen by the majority of the
developers. However there is some value to having this warning. In
some cases there are potential (security) bugs involved where
malicious or simply malformed data from outside the kernel could
trigger stack overflows or other nastyness. Instead of having a
cleanup team running around in perpetuity going over all new format
strings in code it's better to just emit the warning in all cases.

Signed-off-by: Floris Kraak <randakar@gmail.com>
---
diff --git a/Makefile b/Makefile
index 7715b2c..ed3b973 100644
--- a/Makefile
+++ b/Makefile
@@ -346,7 +346,8 @@ KBUILD_CPPFLAGS := -D__KERNEL__

 KBUILD_CFLAGS   := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
 		   -fno-strict-aliasing -fno-common \
-		   -Werror-implicit-function-declaration
+		   -Werror-implicit-function-declaration \
+		   -Wformat-security
 KBUILD_AFLAGS   := -D__ASSEMBLY__

 # Read KERNELRELEASE from include/config/kernel.release (if it exists)
---
"They that give up essential liberty to obtain temporary safety,
deserve neither liberty nor safety."
  -- Ben Franklin

"The course of history shows that as a government grows, liberty
decreases."
  -- Thomas Jefferson

Re: [PATCH] Kbuild: Enable the gcc flag -Wformat-security (was: Re: [PATCH] Kbuild: Disable the -Wformat-security gcc flag)

[Floris Kraak]

On Thu, Feb 5, 2009 at 9:47 AM, Floris Kraak <randakar@gmail.com> wrote:
>
> Here goes:
>

Resend, seems like the earlier patch doesn't quite actually work as
advertised; -Wformat-security needs -Wformat enabled as well.
---
[PATCH] Kbuild: Enable the gcc flag -Wformat-security

Some distributions have enabled the gcc flag -Wformat-security by
default. This causes about 150 new warnings to show up in an
allyesconfig kernel build that are never seen by the majority of the
developers. However there is some value to having this warning. In
some cases there are potential (security) bugs involved where
malicious or simply malformed data from outside the kernel could
trigger stack overflows or other nastyness.
Instead of having a cleanup team running around in perpetuity going
over all new format strings in code it's better to just emit the
warning in all cases.

Signed-off-by: Floris Kraak <randakar@gmail.com>
---
diff --git a/Makefile b/Makefile
index 7715b2c..d186e41 100644
--- a/Makefile
+++ b/Makefile
@@ -346,7 +346,8 @@ KBUILD_CPPFLAGS := -D__KERNEL__

 KBUILD_CFLAGS   := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
 		   -fno-strict-aliasing -fno-common \
-		   -Werror-implicit-function-declaration
+		   -Werror-implicit-function-declaration \
+		   -Wformat -Wformat-security
 KBUILD_AFLAGS   := -D__ASSEMBLY__

 # Read KERNELRELEASE from include/config/kernel.release (if it exists)
---
"They that give up essential liberty to obtain temporary safety,
deserve neither liberty nor safety."
  -- Ben Franklin

"The course of history shows that as a government grows, liberty
decreases."
  -- Thomas Jefferson

Re: [PATCH] Kbuild: Enable the gcc flag -Wformat-security (was: Re: [PATCH] Kbuild: Disable the -Wformat-security gcc flag)

[Sam Ravnborg]

On Thu, Feb 05, 2009 at 09:57:28AM +0100, Floris Kraak wrote:
> On Thu, Feb 5, 2009 at 9:47 AM, Floris Kraak <randakar@gmail.com> wrote:
> >
> > Here goes:
> >
> 
> Resend, seems like the earlier patch doesn't quite actually work as
> advertised; -Wformat-security needs -Wformat enabled as well.
> ---
> [PATCH] Kbuild: Enable the gcc flag -Wformat-security

We need to sort out the ~150 warnings you see in your
allyesconfig build before applying this one.
We try to avoid applying patches that increases the warning count
without some effort done to fix it first.

So let see what the other patch end up with.

	Sam

Re: [PATCH] Kbuild: Enable the gcc flag -Wformat-security (was: Re: [PATCH] Kbuild: Disable the -Wformat-security gcc flag)

[Floris Kraak]

On Thu, Feb 5, 2009 at 8:54 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
> On Thu, Feb 05, 2009 at 09:57:28AM +0100, Floris Kraak wrote:
>> On Thu, Feb 5, 2009 at 9:47 AM, Floris Kraak <randakar@gmail.com> wrote:
>>
>> Resend, seems like the earlier patch doesn't quite actually work as
>> advertised; -Wformat-security needs -Wformat enabled as well.
>> ---
>> [PATCH] Kbuild: Enable the gcc flag -Wformat-security
>
> We need to sort out the ~150 warnings you see in your
> allyesconfig build before applying this one.
> We try to avoid applying patches that increases the warning count
> without some effort done to fix it first.
>
> So let see what the other patch end up with.
>

Fine with me, the other patch needs to cook a bit first tho ;-)

Regards,
Floris
---
"They that give up essential liberty to obtain temporary safety,
deserve neither liberty nor safety."
  -- Ben Franklin

"The course of history shows that as a government grows, liberty
decreases."
  -- Thomas Jefferson