Messages in this thread | | | Date | Mon, 28 Sep 2015 15:30:06 -0700 | Subject | Re: [PATCH 1/4] lib/vsprintf.c: handle invalid format specifiers more robustly | From | Kees Cook <> |
| |
On Fri, Sep 25, 2015 at 10:41 AM, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote: > If we meet any invalid or unsupported format specifier, 'handling' it > by just printing it as a literal string is not safe: Presumably the > format string and the arguments passed gcc's type checking, but that > means something like sprintf(buf, "%n %pd", &intvar, dentry) would end > up interpreting &intvar as a struct dentry*. > > When the offending specifier was %n it used to be at the end of the > format string, but we can't rely on that always being the case. Also, > gcc doesn't complain about some more or less exotic qualifiers (or > 'length modifiers' in posix-speak) such as 'j' or 'q', but being > unrecognized by the kernel's printf implementation, they'd be > interpreted as unknown specifiers, and the rest of arguments would be > interpreted wrongly. > > So let's complain about anything we don't understand, not just %n, and > stop pretending that we'd be able to make sense of the rest of the > format/arguments. If the offending specifier is in a printk() call we > unfortunately only get a "BUG: recent printk recursion!", but at least > direct users of the sprintf family will be caught.
I like it! Thanks :)
Acked-by: Kees Cook <keescook@chromium.org>
-Kees
-- Kees Cook Chrome OS Security
| |