On Fri, Mar 27, 2015 at 6:07 PM, Fengguang Wu <fengguang.wu@xxxxxxxxx> wrote: > + Kees Cook > > On Fri, Mar 27, 2015 at 10:24:53AM -0700, Bryan Wu wrote: >> On Fri, Mar 27, 2015 at 1:09 AM, Ricardo Ribalda Delgado >> <ricardo.ribalda@xxxxxxxxx> wrote: >> > Hi Sakari >> > >> > cc: adding Greg (core and FormatGuard) and Chistopher (sparse) >> >> >> >> I just realised there was another issue --- the name is now interpreted as >> >> format string. Bad things will happen if there's e.g. %s in the name itself >> >> --- perhaps unlikely, but possible. >> > >> > Good catch! >> > >> > Would it be possible to add a sparse check to avoid this in all the kernel? >> > >> > And what about a macro protection like FormatGuard? >> > >> > https://www.usenix.org/legacy/events/sec01/full_papers/cowanbarringer/cowanbarringer.pdf >> > >> > >> >> I think Fengguang's 0-DAY kernel test infrastructure can help this. > > Kees' format-security branch has a check on dynamic printf format > string, which has been effective in finding errors like: > > drivers/tty/serial/sb1250-duart.c: In function 'sbd_map_port': >>> drivers/tty/serial/sb1250-duart.c:680:3: error: format not a string literal and no format arguments [-Werror=format-security] > printk(err); > ^ > > I wonder if Kees has the plan to include the patch into upstream and > make it a kconfig option. For your convenience, the patch is pasted > below. > > Thanks, > Fengguang > --- > > commit 95420c349194d1b570270ba1b1567d85461761c3 > Author: Kees Cook <keescook@xxxxxxxxxxxx> > AuthorDate: Mon Sep 16 11:15:54 2013 -0700 > Commit: Kees Cook <keescook@xxxxxxxxxxxx> > CommitDate: Wed Mar 4 14:07:18 2015 -0800 > > Make all format string problems fail the build > > In an effort to stop format strings from leaking into various callers, > have gcc stop the build when this gets detected. > > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> > > diff --git a/Makefile b/Makefile > index e6a9b1b..b7684d2 100644 > --- a/Makefile > +++ b/Makefile > @@ -402,7 +402,6 @@ KBUILD_CPPFLAGS := -D__KERNEL__ > KBUILD_CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ > -fno-strict-aliasing -fno-common \ > -Werror-implicit-function-declaration \ > - -Wno-format-security \ > -std=gnu89 > > KBUILD_AFLAGS_KERNEL := > @@ -752,6 +751,11 @@ endif > NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include) > CHECKFLAGS += $(NOSTDINC_FLAGS) > > +# Enable format-security when it can stop the build, otherwise disable. > +KBUILD_CFLAGS += $(call cc-option,\ > + -Wformat -Wformat-security -Werror=format-security,\ > + -Wno-format-security) > + > # warn about C99 declaration after statement > KBUILD_CFLAGS += $(call cc-option,-Wdeclaration-after-statement,) > The trouble is that gcc still has a terrible time with false positives on these warnings. It doesn't know to silence const char strings. For example: const char * version = "SomeThing driver v1.3\n"; ... printk(version); We don't care about this, since the string is not dynamic, but gcc still warns. My intention is that when gcc fixes this bug, then I'd upstream this patch. Right now I have to carry a patch to silence false positives. :( You can see it here: http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/log/?h=format-security -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-leds" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
