summaryrefslogtreecommitdiffstats
path: root/lib
diff options
context:
space:
mode:
authorRasmus Villemoes <linux@rasmusvillemoes.dk>2015-11-06 16:30:20 -0800
committerLinus Torvalds <torvalds@linux-foundation.org>2015-11-06 17:50:42 -0800
commitb006f19b055f90b73e97086490f95b83095dcc91 (patch)
treeea21a1b1720b1ba8269f0914048785a47792c253 /lib
parent5e4ee7b13b522d07196e737f399843c58569604d (diff)
downloadlinux-b006f19b055f90b73e97086490f95b83095dcc91.tar.gz
linux-b006f19b055f90b73e97086490f95b83095dcc91.tar.bz2
linux-b006f19b055f90b73e97086490f95b83095dcc91.zip
lib/vsprintf.c: handle invalid format specifiers more robustly
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. Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Acked-by: Kees Cook <keescook@chromium.org> Cc: Martin Kletzander <mkletzan@redhat.com> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'lib')
-rw-r--r--lib/vsprintf.c31
1 files changed, 21 insertions, 10 deletions
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index e966a45e2f00..e35724c2b2a8 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1772,14 +1772,14 @@ qualifier:
case 'n':
/*
- * Since %n poses a greater security risk than utility, treat
- * it as an invalid format specifier. Warn about its use so
- * that new instances don't get added.
+ * Since %n poses a greater security risk than
+ * utility, treat it as any other invalid or
+ * unsupported format specifier.
*/
- WARN_ONCE(1, "Please remove ignored %%n in '%s'\n", fmt);
/* Fall-through */
default:
+ WARN_ONCE(1, "Please remove unsupported %%%c in format string\n", *fmt);
spec->type = FORMAT_TYPE_INVALID;
return fmt - start;
}
@@ -1920,10 +1920,15 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
break;
case FORMAT_TYPE_INVALID:
- if (str < end)
- *str = '%';
- ++str;
- break;
+ /*
+ * Presumably the arguments passed gcc's type
+ * checking, but there is no safe or sane way
+ * for us to continue parsing the format and
+ * fetching from the va_list; the remaining
+ * specifiers and arguments would be out of
+ * sync.
+ */
+ goto out;
default:
switch (spec.type) {
@@ -1968,6 +1973,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
}
}
+out:
if (size > 0) {
if (str < end)
*str = '\0';
@@ -2165,9 +2171,10 @@ do { \
switch (spec.type) {
case FORMAT_TYPE_NONE:
- case FORMAT_TYPE_INVALID:
case FORMAT_TYPE_PERCENT_CHAR:
break;
+ case FORMAT_TYPE_INVALID:
+ goto out;
case FORMAT_TYPE_WIDTH:
case FORMAT_TYPE_PRECISION:
@@ -2229,6 +2236,7 @@ do { \
}
}
+out:
return (u32 *)(PTR_ALIGN(str, sizeof(u32))) - bin_buf;
#undef save_arg
}
@@ -2351,12 +2359,14 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
break;
case FORMAT_TYPE_PERCENT_CHAR:
- case FORMAT_TYPE_INVALID:
if (str < end)
*str = '%';
++str;
break;
+ case FORMAT_TYPE_INVALID:
+ goto out;
+
default: {
unsigned long long num;
@@ -2399,6 +2409,7 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
} /* switch(spec.type) */
} /* while(*fmt) */
+out:
if (size > 0) {
if (str < end)
*str = '\0';