Re: [RFC PATCH 0/7] runtime format string checking

From: Kees Cook <keescook@chromium.org>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH 0/7] runtime format string checking
Date: Fri, 2 Nov 2018 13:46:40 -0700	[thread overview]
Message-ID: <CAGXu5jKas+bWbqS9juAYrQzbgGq8U08Q91dy0zdmkSZDU++rag@mail.gmail.com> (raw)
In-Reply-To: <22cb7c93-1160-b181-5702-1d7bfcee5b18@rasmusvillemoes.dk>

On Fri, Nov 2, 2018 at 1:09 PM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> That's a bit too naive. At the very least, you must exclude static
> stuff, i.e. restrict to actual auto variables. Otherwise you're making
> things worse (a "static const char []" just occupies some space in
> .rodata, a "static const char * const" occupies the same space for the
> anonymous literal, plus space for a pointer). Furthermore, you must
> ensure that nobody does sizeof() on VAR. With a trivial extension of
> your script to exclude the "static const char" places, I get

Yes, thank you. That's the part I was forgetting and why I was doing
[] over * back then. There are certainly uses of sizeof() on these
strings. So, it seems better to get sizeof() right that the const-ness
right.

>>>> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=kspp/format-security&id=b7dcfc8f48caaafcc423e5793f7ef61b9bb5c458
>>>> This one covers cases where the pointer is pointing to a const string,
>>>> so really there's no sense in injecting the "%s", but I was collecting
>>>> them to make real ones stand out.
>>>
>>> I don't agree. [...]
>>
>> Okay, then I'll forward this to akpm maybe?
>
> Yes, if all they do is replace f(..., s) by f(..., "%s", s) that should
> never hurt. Maybe check if there's a ..._puts() variant that can be used
> instead, e.g. seq_puts().

Alright, I'll see about bringing that series forward in time...

-Kees

-- 
Kees Cook