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

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: <CAGXu5jKas+bWbqS9juAYrQzbgGq8U08Q91dy0zdmkSZDU++rag@mail.gmail.com>
Date:   Fri, 2 Nov 2018 13:46:40 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 0/7] runtime format string checking

On Fri, Nov 2, 2018 at 1:09 PM, Rasmus Villemoes
<linux@...musvillemoes.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