|
Message-ID: <CAGXu5jLvFbfjYUuZnD0xK-9sgLbb-XPvTp1ieoTrMh_CNj0H-w@mail.gmail.com> 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.