linux-kernel - Re: [PATCH] iwl4965: Enable checking of format strings

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]

Message-ID: <87y4o2xfgz.fsf@rasmusvillemoes.dk>
Date:	Fri, 13 Feb 2015 11:39:40 +0100
From:	Rasmus Villemoes <linux@...musvillemoes.dk>
To:	Mark Rustad <mrustad@...il.com>
Cc:	"Rustad\, Mark D" <mark.d.rustad@...el.com>,
	Stanislaw Gruszka <sgruszka@...hat.com>,
	Kalle Valo <kvalo@...eaurora.org>,
	"linux-wireless\@vger.kernel.org" <linux-wireless@...r.kernel.org>,
	"netdev\@vger.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] iwl4965: Enable checking of format strings

On Fri, Feb 13 2015, Mark Rustad <mrustad@...il.com> wrote:

> On 2/12/15 2:20 AM, Rasmus Villemoes wrote:
>> Rather weak arguments, but I have three of them :-)
>
> Yes, weak. All three.
>
>> (1) If I'm reading some code and spot a non-constant format
>> argument, I sometimes track back to see how e.g. fmt_value is
>> defined. If I then see it's a macro, I immediately think "ok, the
>> compiler is doing type-checking". If it is a const char[], I have
>> to remember that gcc also does it in that case (as opposed to for
>> example const char*const).
>
> GCC should check in both cases. The case you are replacing was not
> const char * const, but only const char *. Still, the compiler really
> should check either form, even though theoretically the pointer in the
> latter case could be changed, but the initial const value should be a
> good indication of what the parameters are expected to be. No real
> reason for the compiler not to check it.

I agree with all of that - just wanted to point out what gcc currently
does and doesn't, and changing to const char[] would indeed enable
checking.

>> (2) The names of these variables themselves may end up wasting a
>> few bytes in the image.
>
> Maybe in a debug image, but they should be stripped from any normal
> image. Really not a factor.

Sure, that was by far the weakest, and let's ignore that.

>> (3) gcc/the linker doesn't merge identical const char[] arrays
>> across translation units. It also doesn't consider their tails for
>> merging with string literals. So although these specific strings
>> are unlikely to appear elsewhere, a string such as "%10u\n" or
>> "max\n" couldn't be merged with one of the above.
>
> I haven't checked, but there is no theoretical reason that const char
> [] items could not be merged exactly as the literals are. Considering
> the boundaries the compiler guys push on optimization, doing such
> merging would be tame by comparison (speculative stores make me crazy).

Well, probably the linker is allowed to overlap "anonymous" objects
(string literals) with whatever const char[] (or indeed any const)
object it finds containing the appropriate byte sequence. But I think
language lawyers would insist that for

const char foo[] = "a string";
const char bar[] = "a string";

foo and bar have different addresses, whether they are defined in the
same or different TUs. One could then argue that if their address is
never taken explicitly it should be ok, but since passing foo to a printf
function effectively makes the address of foo escape the TU (even though
one is formally passing pointer to first element), I can certainly see
why compiler people would be reluctant to do merging of such objects.

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