linux-kernel - Re: [PATCH]: Cleanup: Remove gcc format string warnings when compiling with -Wformat-security

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 05 Feb 2009 17:42:52 -0600
From:	Robert Hancock <hancockrwd@...il.com>
To:	Roland Dreier <rdreier@...co.com>
CC:	Floris Kraak <randakar@...il.com>, Sam Ravnborg <sam@...nborg.org>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Trivial Patch Monkey <trivial@...nel.org>,
	Andreas Schwab <schwab@...e.de>
Subject: Re: [PATCH]: Cleanup: Remove gcc format string warnings when compiling
  with -Wformat-security

Roland Dreier wrote:
>  > Here's the patch that I get when I blindly patch every single location
>  > that emits this warning.
> 
> I would strongly prefer to do this with a little more care.  For example
> the b43/main.c change:
> 
>  > --- a/drivers/net/wireless/b43/main.c
>  > +++ b/drivers/net/wireless/b43/main.c
>  > @@ -2005,9 +2005,9 @@ static void b43_print_fw_helptext(struct b43_wl
>  > *wl, bool error)
>  >  	       "http://linuxwireless.org/en/users/Drivers/b43#devicefirmware "
>  >  	       "and download the latest firmware (version 4).\n";
>  >  	if (error)
>  > -		b43err(wl, text);
>  > +		b43err(wl, "%s", text);
>  >  	else
>  > -		b43warn(wl, text);
>  > +		b43warn(wl, "%s", text);
>  >  }
> would probably be better solved by doing
> 
> --- a/drivers/net/wireless/b43/main.c
> +++ b/drivers/net/wireless/b43/main.c
> @@ -1999,11 +1999,11 @@ static void b43_release_firmware(struct b43_wldev *dev)
>  
>  static void b43_print_fw_helptext(struct b43_wl *wl, bool error)
>  {
> -	const char *text;
> +	static const char text[] =
> +		"You must go to "
> +		"http://linuxwireless.org/en/users/Drivers/b43#devicefirmware "
> +		"and download the latest firmware (version 4).\n";
>  
> -	text = "You must go to "
> -	       "http://linuxwireless.org/en/users/Drivers/b43#devicefirmware "
> -	       "and download the latest firmware (version 4).\n";
>  	if (error)
>  		b43err(wl, text);
>  	else

"const char* const" text would likely work as well..

> 
> and in any case I'm not totally convinced that we want to add the bloat
> for trivial cases like
> 
> 	char *safe = "foo";
> 	printk(safe);

Well, in that case, printk("foo") would be the obvious solution :-)

> 
> Would be nice to think of a cleverer way to handle that...
> 
>  - R.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/