linux-kernel - Re: [PATCH] Kbuild: Disable the -Wformat-security gcc flag

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, 5 Feb 2009 11:15:12 +0100
From:	Floris Kraak <randakar@...il.com>
To:	Roland Dreier <rdreier@...co.com>
Cc:	Robert Hancock <hancockrwd@...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>
Subject: Re: [PATCH] Kbuild: Disable the -Wformat-security gcc flag

On Thu, Feb 5, 2009 at 9:26 AM, Floris Kraak <randakar@...il.com> wrote:
> On Thu, Feb 5, 2009 at 7:37 AM, Roland Dreier <rdreier@...co.com> wrote:
>>  > Just how many of these warnings are showing up? In the cases you
>>  > posted it's presumably no problem, but if the string could either a)
>>  > be potentially set by a malicious user or b) accidentally contain
>>  > printk format characters then this code has a risk that things could
>>  > blow up..
>>
>> I get ~150 of them on an x86 allyesconfig build here (see below).  Many
>> but not all are trivial; some at least appear to be passing in strings
>> that come from random hardware/firmware or DNS names etc (ie there's at
>> least a chance of a '%'); and I didn't exhaustively audit to make sure
>> none of them could print something from an unprivileged user.
>>
>
> There are probably some real bugs in there. On the other hand there is
> some overhead to fixing the warnings. Kernel text size increase,
> possibly some CPU overhead from parsing the format string. Hopefully
> none of these calls are in really hot code paths ;-)
> 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.
>
> What remains an open question to me though is what to do with cases
> where the warning not only can be ignored but literally should be. eg.
> when there is zero chance of something unexpected getting passed in
> and 'fixing' it would just bloat the kernel.
> Can sparse be used to check this kind of thing for correctness?
>

Example:

kernel/power/main.c:717: warning: format not a string literal and no
format arguments

This complains about:
..
        if (!rtc) {
                printk(warn_no_rtc);
                goto done;
        }
..

So what is this "warn_no_rtc" thing?

        static char             warn_no_rtc[] __initdata =
                KERN_WARNING "PM: no wakealarm-capable RTC driver is ready\n";

That's pretty much GCC failing to recognize that the format is a
string literal and then complaining that it isn't.
How do we make the warning go away without growing the kernel text?
Given the use of __initdata flags I'm not even sure if doing the
obvious printk("%s", warn_no_rtc) isn't going to introduce a subtle
bug somehow..

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