[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Wed, 7 Dec 2016 14:08:40 -0800
From: Kees Cook <keescook@...omium.org>
To: "kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>
Subject: Re: Picking "Write a plugin to do format string
warnings correctly"
On Tue, Dec 6, 2016 at 3:21 PM, Ruslan Kuprieiev <kupruser@...il.com> wrote:
> Hi!
>
> After watching a bunch of talks from Kees about security, I've finally
> decided to try to participate in KSPP.
Hi and welcome!
> If it's not taken, I would like to start with this task:
>
> Write a plugin to do format string warnings correctly (gcc's
> -Wformat-security is bad about const strings)
>
> Unfortunately, I wasn't able to find any details about this task. Could
> someone provide some info about it, please?
Sorry those TODOs items are so terse; they're mostly just brain dumps
so we don't lose things. :)
The problem with -Wformat-security in gcc is that it doesn't correctly
notice a bunch of const string cases. If you look at my
format-security branch:
http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/log/?h=format-security
There's a commit at the top called "format-security: move static
strings to const", which illustrates the first kind of problem gcc
has, which is not realizing that "const char *var" is "const char
var[]":
- const char *name = "fbcon";
+ const char name[] = "fbcon";
This can be seen with:
int main(int argc, char * argv[])
{
char cmd[80];
const char *pointer = "pointer: %p\n";
snprintf(cmd, sizeof(cmd), "cat /proc/%d/maps", getpid());
system(cmd);
printf(pointer, pointer);
fflush(NULL);
pointer[0] = 'P';
return 0;
}
$ gcc -Wall -Wformat=2 -Wformat-security format-security.c -o format-security
format-security.c: In function ‘main’:
format-security.c:22:5: warning: format not a string literal, argument
types not checked [-Wformat-nonliteral]
printf(pointer, pointer);
^
format-security.c:24:16: error: assignment of read-only location ‘*pointer’
pointer[0] = 'P';
^
And, this is right, if line 24 is removed, we can see the contents of
"pointer" are, as expected, in read-only memory:
$ ./format-security
55aa9d70b000-55aa9d70c000 r-xp 00000000 fd:06 28
/tmp/format-security
55aa9d90b000-55aa9d90c000 r--p 00000000 fd:06 28
/tmp/format-security
55aa9d90c000-55aa9d90d000 rw-p 00001000 fd:06 28
/tmp/format-security
...
pointer: 0x55aa9d70b984
The reason gcc DOES warn about "const char *pointer" is because the
value of pointer itself can change. Building with "const char * const
pointer" tells gcc "pointer" should be read-only (though usually this
isn't enforced in variable placement). The goal would be to teach gcc
that if a variable is only ever assigned to read-only locations, it's
a false positive, so don't warn about it.
Here's an example from the kernel, in drivers/gpu/drm/ttm/ttm_memory.c:
static int ttm_mem_init_kernel_zone(struct ttm_mem_global *glob,
const struct sysinfo *si)
{
struct ttm_mem_zone *zone = kzalloc(sizeof(*zone), GFP_KERNEL);
...
zone->name = "kernel";
...
ret = kobject_init_and_add(
&zone->kobj, &ttm_mem_zone_kobj_type, &glob->kobj, zone->name);
zone->name is pointing to a read-only string. -Wformat-security yells
about this because it can't tell that ->name is never reassigned
between the first place and where it gets used as a format string.
-Kees
--
Kees Cook
Nexus Security
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
