A common error is to pass a "char" or "signed char" to %02x (or
%.2X or some other variant). This can actually be a security
problem, because a lot of code expects this to produce exactly two
characters of output.
Unfortunately this also produces false positives, since we're
sometimes in arch-specific code on an arch where char is always
unsigned. I think it would be better, even in those cases, to do an
explicit '& 0xff' or equivalently cast the argument to (u8), to help
readers and static checkers not worry and prevent copy-pasting from
introducing an actual bug (and gcc should be able to ignore the mask
if char is indeed unsigned), but evidently not all maintainers agree,
so we'll have to live with these.
Signed-off-by: Rasmus Villemoes <rv@xxxxxxxxxxxxxxxxxx>
---
check_kernel_printf.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/check_kernel_printf.c b/check_kernel_printf.c
index f7cd2fa..88aeee8 100644
--- a/check_kernel_printf.c
+++ b/check_kernel_printf.c
@@ -693,6 +693,38 @@ pointer(const char *fmt, struct expression *arg, int vaidx)
}
}
+/*
+ * A common error is to pass a "char" or "signed char" to %02x (or
+ * %.2X or some other variant). This can actually be a security
+ * problem, because a lot of code expects this to produce exactly two
+ * characters of output. Unfortunately this also produces false
+ * positives, since we're sometimes in arch-specific code on an arch
+ * where char is always unsigned.
+ */
+static void
+hexbyte(const char *fmt, int fmt_len, struct expression *arg, int vaidx, struct printf_spec spec)
+{
+ struct symbol *type;
+
+ /*
+ * For now, just check the most common and obvious, which is
+ * roughly %[.0]2[xX].
+ */
+ if (spec.field_width != 2 && spec.precision != 2)
+ return;
+ if (spec.base != 16)
+ return;
+
+ type = get_type(arg);
+ if (!type) {
+ sm_msg("warn: could not determine type of argument %d", vaidx);
+ return;
+ }
+ if (type == &char_ctype || type == &schar_ctype)
+ sm_msg("warn: argument %d to %.*s specifier has type '%s'",
+ vaidx, fmt_len, fmt, type_to_str(type));
+}
+
static int
check_format_string(const char *fmt, const char *caller)
{
@@ -934,6 +966,7 @@ do_check_printf_call(const char *caller, const char *name, struct expression *ca
break;
case FORMAT_TYPE_UINT:
+ hexbyte(old_fmt, fmt-old_fmt, arg, vaidx, spec);
case FORMAT_TYPE_LONG:
case FORMAT_TYPE_ULONG:
case FORMAT_TYPE_LONG_LONG:
--
2.6.1
--
To unsubscribe from this list: send the line "unsubscribe smatch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
