[PATCH 2/2] vsprintf: Stop using obsolete simple_strtoul()

Thomas Preston
2018-12-11T15:21:34Z

Stop using the obsolete functions simple_strtoul() and
simple_strtoull(). Instead, we should use the improved kstrtol() and
kstrtoll() functions. To do this, we must copy the current field into a
null-terminated tmpstr and advance the variable `next` manually.

The width of tmpstr has been chosen because no integer field can be
larger than 22 characters (ULLONG_MAX in octal), plus sign, radix,
new-line, null-terminator and some extra for alignment.

This patch fixes a bug with vsscanf. If passing sscan a 16-digit
hex-string like so:

sscanf("fafafafa0b0b0b0b", "%8x%8x", &hi, &lo)

then the result comes out with hi always being 0.

The issue is that the code calls simple_strtoul() which consumes up
to 16-digits but only returns an unsigned long (8 hex digits on ARM).
The vsscanf() code then checks and finds that the field_width of 8 was
greater than the 16 consumed characters and tries to fix this by:

while (next - str > field_width) {
if (is_sign)
val.s = div_s64(val.s, base);
else
val.u = div_u64(val.u, base);
--next;
}

However val.{s,u} is already trunacted from the simple_strtoul call
and all that happens is the value gets divided down to zero.

Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
lib/vsprintf.c | 36 ++++++++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index bbf2ac734711..ec23e18e8cc6 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -47,6 +47,8 @@
#include <linux/string_helpers.h>
#include "kstrtox.h"

+#define INT_BUF_LEN 28
+
/**
* simple_strtoull - convert a string to an unsigned long long
* @cp: The start of the string
@@ -2914,7 +2916,9 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
u8 qualifier;
unsigned int base;
union {
+ long sl;
long long sll;
+ unsigned long ul;
unsigned long long ull;
} val;
s16 field_width;
@@ -3119,14 +3123,30 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
|| (base == 0 && !isdigit(digit)))
break;

- if (is_sign)
- val.sll = qualifier != 'L' ?
- simple_strtol(str, &next, base) :
- simple_strtoll(str, &next, base);
- else
- val.ull = qualifier != 'L' ?
- simple_strtoul(str, &next, base) :
- simple_strtoull(str, &next, base);
+ if (unlikely((field_width+1) > INT_BUF_LEN))
+ return num;
+
+ if (field_width > 0) {
+ char tmpstr[INT_BUF_LEN];
+ int ret;
+
+ strscpy(tmpstr, str, field_width+1);
+
+ if (is_sign)
+ if (qualifier != 'L')
+ ret = kstrtol(tmpstr, base, &val.sl);
+ else
+ ret = kstrtoll(tmpstr, base, &val.sll);
+ else
+ if (qualifier != 'L')
+ ret = kstrtoul(tmpstr, base, &val.ul);
+ else
+ ret = kstrtoull(tmpstr, base, &val.ull);
+ if (ret < 0)
+ return num;
+
+ }
+ next = (char *)str + field_width;

if (field_width > 0 && next - str > field_width) {
if (base == 0)
--
2.11.0