[PATCH 01/13] lib/vsprintf.c: pull out padding code from dentry_name()

[PATCH 01/13] lib/vsprintf.c: pull out padding code from dentry_name()

[Rasmus Villemoes]

Pull out the logic in dentry_name() which handles field width space
padding, in preparation for reusing it from string(). Rename the
widen() helper to move_right(), since it is used for handling the
!(flags & LEFT) case.

Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/vsprintf.c | 46 +++++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 98b0d7be3fb7..0edf8dd0518f 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -538,7 +538,7 @@ char *string(char *buf, char *end, const char *s, struct printf_spec spec)
 	return buf;
 }
 
-static void widen(char *buf, char *end, unsigned len, unsigned spaces)
+static void move_right(char *buf, char *end, unsigned len, unsigned spaces)
 {
 	size_t size;
 	if (buf >= end)	/* nowhere to put anything */
@@ -556,6 +556,35 @@ static void widen(char *buf, char *end, unsigned len, unsigned spaces)
 	memset(buf, ' ', spaces);
 }
 
+/*
+ * Handle field width padding for a string.
+ * @buf: current buffer position
+ * @n: length of string
+ * @end: end of output buffer
+ * @spec: for field width and flags
+ * Returns: new buffer position after padding.
+ */
+static noinline_for_stack
+char *widen_string(char *buf, int n, char *end, struct printf_spec spec)
+{
+	unsigned spaces;
+
+	if (likely(n >= spec.field_width))
+		return buf;
+	/* we want to pad the sucker */
+	spaces = spec.field_width - n;
+	if (!(spec.flags & LEFT)) {
+		move_right(buf - n, end, n, spaces);
+		return buf + spaces;
+	}
+	while (spaces--) {
+		if (buf < end)
+			*buf = ' ';
+		++buf;
+	}
+	return buf;
+}
+
 static noinline_for_stack
 char *dentry_name(char *buf, char *end, const struct dentry *d, struct printf_spec spec,
 		  const char *fmt)
@@ -597,20 +626,7 @@ char *dentry_name(char *buf, char *end, const struct dentry *d, struct printf_sp
 			*buf = c;
 	}
 	rcu_read_unlock();
-	if (n < spec.field_width) {
-		/* we want to pad the sucker */
-		unsigned spaces = spec.field_width - n;
-		if (!(spec.flags & LEFT)) {
-			widen(buf - n, end, n, spaces);
-			return buf + spaces;
-		}
-		while (spaces--) {
-			if (buf < end)
-				*buf = ' ';
-			++buf;
-		}
-	}
-	return buf;
+	return widen_string(buf, n, end, spec);
 }
 
 static noinline_for_stack
-- 
2.6.1

[PATCH 02/13] lib/vsprintf.c: move string() below widen_string()

[Rasmus Villemoes]

This is pure code movement, making sure the widen_string() helper is
defined before the string() function.

Cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/vsprintf.c | 62 +++++++++++++++++++++++++++++-----------------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 0edf8dd0518f..a0145b5d114e 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -507,37 +507,6 @@ char *number(char *buf, char *end, unsigned long long num,
 	return buf;
 }
 
-static noinline_for_stack
-char *string(char *buf, char *end, const char *s, struct printf_spec spec)
-{
-	int len, i;
-
-	if ((unsigned long)s < PAGE_SIZE)
-		s = "(null)";
-
-	len = strnlen(s, spec.precision);
-
-	if (!(spec.flags & LEFT)) {
-		while (len < spec.field_width--) {
-			if (buf < end)
-				*buf = ' ';
-			++buf;
-		}
-	}
-	for (i = 0; i < len; ++i) {
-		if (buf < end)
-			*buf = *s;
-		++buf; ++s;
-	}
-	while (len < spec.field_width--) {
-		if (buf < end)
-			*buf = ' ';
-		++buf;
-	}
-
-	return buf;
-}
-
 static void move_right(char *buf, char *end, unsigned len, unsigned spaces)
 {
 	size_t size;
@@ -586,6 +555,37 @@ char *widen_string(char *buf, int n, char *end, struct printf_spec spec)
 }
 
 static noinline_for_stack
+char *string(char *buf, char *end, const char *s, struct printf_spec spec)
+{
+	int len, i;
+
+	if ((unsigned long)s < PAGE_SIZE)
+		s = "(null)";
+
+	len = strnlen(s, spec.precision);
+
+	if (!(spec.flags & LEFT)) {
+		while (len < spec.field_width--) {
+			if (buf < end)
+				*buf = ' ';
+			++buf;
+		}
+	}
+	for (i = 0; i < len; ++i) {
+		if (buf < end)
+			*buf = *s;
+		++buf; ++s;
+	}
+	while (len < spec.field_width--) {
+		if (buf < end)
+			*buf = ' ';
+		++buf;
+	}
+
+	return buf;
+}
+
+static noinline_for_stack
 char *dentry_name(char *buf, char *end, const struct dentry *d, struct printf_spec spec,
 		  const char *fmt)
 {
-- 
2.6.1

[PATCH 03/13] lib/vsprintf.c: eliminate potential race in string()

[Rasmus Villemoes]

If the string corresponding to a %s specifier can change under us, we
might end up copying a \0 byte to the output buffer. There might be
callers who expect the output buffer to contain a genuine C string
whose length is exactly the snprintf return value (assuming truncation
hasn't happened or has been checked for).

We can avoid this by only passing over the source string once,
stopping the first time we meet a nul byte (or when we reach the given
precision), and then letting widen_string() handle left/right space
padding.

Cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/vsprintf.c | 28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index a0145b5d114e..ba00eab1e703 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -557,32 +557,22 @@ char *widen_string(char *buf, int n, char *end, struct printf_spec spec)
 static noinline_for_stack
 char *string(char *buf, char *end, const char *s, struct printf_spec spec)
 {
-	int len, i;
+	int len = 0;
+	size_t lim = spec.precision;
 
 	if ((unsigned long)s < PAGE_SIZE)
 		s = "(null)";
 
-	len = strnlen(s, spec.precision);
-
-	if (!(spec.flags & LEFT)) {
-		while (len < spec.field_width--) {
-			if (buf < end)
-				*buf = ' ';
-			++buf;
-		}
-	}
-	for (i = 0; i < len; ++i) {
-		if (buf < end)
-			*buf = *s;
-		++buf; ++s;
-	}
-	while (len < spec.field_width--) {
+	while (lim--) {
+		char c = *s++;
+		if (!c)
+			break;
 		if (buf < end)
-			*buf = ' ';
+			*buf = c;
 		++buf;
+		++len;
 	}
-
-	return buf;
+	return widen_string(buf, len, end, spec);
 }
 
 static noinline_for_stack
-- 
2.6.1

[PATCH 04/13] lib/vsprintf.c: expand field_width to 24 bits

[Rasmus Villemoes]

Maurizio Lombardi reported a problem [1] with the %pb extension: It
doesn't work for sufficiently large bitmaps, since the size is stashed
in the field_width field of the struct printf_spec, which is currently
an s16. Concretely, this manifested itself in
/sys/bus/pseudo/drivers/scsi_debug/map being empty, since the bitmap
printer got a size of 0, which is the 16 bit truncation of the actual
bitmap size.

We do want to keep struct printf_spec at 8 bytes so that it can
cheaply be passed by value. The qualifier field is only used for
internal bookkeeping in format_decode, so we might as well use a local
variable for that. This gives us an additional 8 bits, which we can
then use for the field width.

To stay in 8 bytes, we need to do a little rearranging and make the
type member a bitfield as well. For consistency, change all the
members to bit fields. gcc doesn't generate much worse code with these
changes (in fact, bloat-o-meter says we save 300 bytes - which I think
is a little surprising).

I didn't find a BUILD_BUG/compiletime_assertion/... which would work
outside function context, so for now I just open-coded it.

[1] http://thread.gmane.org/gmane.linux.kernel/2034835

Reported-by: Maurizio Lombardi <mlombard@redhat.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/vsprintf.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index ba00eab1e703..373da7a84382 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -380,13 +380,13 @@ enum format_type {
 };
 
 struct printf_spec {
-	u8	type;		/* format_type enum */
-	u8	flags;		/* flags to number() */
-	u8	base;		/* number base, 8, 10 or 16 only */
-	u8	qualifier;	/* number qualifier, one of 'hHlLtzZ' */
-	s16	field_width;	/* width of output field */
-	s16	precision;	/* # of digits/chars */
+	unsigned int	type:8;		/* format_type enum */
+	signed int	field_width:24;	/* width of output field */
+	unsigned int	flags:8;	/* flags to number() */
+	unsigned int	base:8;		/* number base, 8, 10 or 16 only */
+	signed int	precision:16;	/* # of digits/chars */
 };
+extern char __check_printf_spec[1-2*(sizeof(struct printf_spec) != 8)];
 
 static noinline_for_stack
 char *number(char *buf, char *end, unsigned long long num,
@@ -1639,6 +1639,7 @@ static noinline_for_stack
 int format_decode(const char *fmt, struct printf_spec *spec)
 {
 	const char *start = fmt;
+	char qualifier;
 
 	/* we finished early by reading the field width */
 	if (spec->type == FORMAT_TYPE_WIDTH) {
@@ -1721,16 +1722,16 @@ precision:
 
 qualifier:
 	/* get the conversion qualifier */
-	spec->qualifier = -1;
+	qualifier = 0;
 	if (*fmt == 'h' || _tolower(*fmt) == 'l' ||
 	    _tolower(*fmt) == 'z' || *fmt == 't') {
-		spec->qualifier = *fmt++;
-		if (unlikely(spec->qualifier == *fmt)) {
-			if (spec->qualifier == 'l') {
-				spec->qualifier = 'L';
+		qualifier = *fmt++;
+		if (unlikely(qualifier == *fmt)) {
+			if (qualifier == 'l') {
+				qualifier = 'L';
 				++fmt;
-			} else if (spec->qualifier == 'h') {
-				spec->qualifier = 'H';
+			} else if (qualifier == 'h') {
+				qualifier = 'H';
 				++fmt;
 			}
 		}
@@ -1787,19 +1788,19 @@ qualifier:
 		return fmt - start;
 	}
 
-	if (spec->qualifier == 'L')
+	if (qualifier == 'L')
 		spec->type = FORMAT_TYPE_LONG_LONG;
-	else if (spec->qualifier == 'l') {
+	else if (qualifier == 'l') {
 		BUILD_BUG_ON(FORMAT_TYPE_ULONG + SIGN != FORMAT_TYPE_LONG);
 		spec->type = FORMAT_TYPE_ULONG + (spec->flags & SIGN);
-	} else if (_tolower(spec->qualifier) == 'z') {
+	} else if (_tolower(qualifier) == 'z') {
 		spec->type = FORMAT_TYPE_SIZE_T;
-	} else if (spec->qualifier == 't') {
+	} else if (qualifier == 't') {
 		spec->type = FORMAT_TYPE_PTRDIFF;
-	} else if (spec->qualifier == 'H') {
+	} else if (qualifier == 'H') {
 		BUILD_BUG_ON(FORMAT_TYPE_UBYTE + SIGN != FORMAT_TYPE_BYTE);
 		spec->type = FORMAT_TYPE_UBYTE + (spec->flags & SIGN);
-	} else if (spec->qualifier == 'h') {
+	} else if (qualifier == 'h') {
 		BUILD_BUG_ON(FORMAT_TYPE_USHORT + SIGN != FORMAT_TYPE_SHORT);
 		spec->type = FORMAT_TYPE_USHORT + (spec->flags & SIGN);
 	} else {
-- 
2.6.1

Re: [PATCH 04/13] lib/vsprintf.c: expand field_width to 24 bits

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1484 bytes --]

Hi Rasmus,

[auto build test ERROR on next-20151020 -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Rasmus-Villemoes/lib-vsprintf-c-pull-out-padding-code-from-dentry_name/20151021-043621
config: frv-defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=frv 

All errors (new ones prefixed by >>):

>> lib/vsprintf.c:389:13: error: size of array '__check_printf_spec' is negative
    extern char __check_printf_spec[1-2*(sizeof(struct printf_spec) != 8)];
                ^

vim +/__check_printf_spec +389 lib/vsprintf.c

   383		unsigned int	type:8;		/* format_type enum */
   384		signed int	field_width:24;	/* width of output field */
   385		unsigned int	flags:8;	/* flags to number() */
   386		unsigned int	base:8;		/* number base, 8, 10 or 16 only */
   387		signed int	precision:16;	/* # of digits/chars */
   388	};
 > 389	extern char __check_printf_spec[1-2*(sizeof(struct printf_spec) != 8)];
   390	
   391	static noinline_for_stack
   392	char *number(char *buf, char *end, unsigned long long num,

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 7992 bytes --]

Re: [PATCH 04/13] lib/vsprintf.c: expand field_width to 24 bits

[Rasmus Villemoes]

On Wed, Oct 21 2015, kbuild test robot <lkp@intel.com> wrote:

> Hi Rasmus,
>
> [auto build test ERROR on next-20151020 -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
>
> url:    https://github.com/0day-ci/linux/commits/Rasmus-Villemoes/lib-vsprintf-c-pull-out-padding-code-from-dentry_name/20151021-043621
> config: frv-defconfig (attached as .config)
> reproduce:
>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=frv 

OK, apparently we need to tell gcc explicitly to pack it on frv. I've
reproduced using the above instructions, and adding __packed solves
it. Please use the alternative patch I'll send as a follow-up in a
moment.

I have to say, this automated build testing before the patches hit
-next (indeed, almost before git send-email is finished sending out the
entire series!) is rather awesome.

Rasmus

[PATCH v2 04/13] lib/vsprintf.c: expand field_width to 24 bits

[Rasmus Villemoes]

Maurizio Lombardi reported a problem [1] with the %pb extension: It
doesn't work for sufficiently large bitmaps, since the size is stashed
in the field_width field of the struct printf_spec, which is currently
an s16. Concretely, this manifested itself in
/sys/bus/pseudo/drivers/scsi_debug/map being empty, since the bitmap
printer got a size of 0, which is the 16 bit truncation of the actual
bitmap size.

We do want to keep struct printf_spec at 8 bytes so that it can
cheaply be passed by value. The qualifier field is only used for
internal bookkeeping in format_decode, so we might as well use a local
variable for that. This gives us an additional 8 bits, which we can
then use for the field width.

To stay in 8 bytes, we need to do a little rearranging and make the
type member a bitfield as well. For consistency, change all the
members to bit fields. gcc doesn't generate much worse code with these
changes (in fact, bloat-o-meter says we save 300 bytes - which I think
is a little surprising).

I didn't find a BUILD_BUG/compiletime_assertion/... which would work
outside function context, so for now I just open-coded it.

[1] http://thread.gmane.org/gmane.linux.kernel/2034835

Reported-by: Maurizio Lombardi <mlombard@redhat.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
v2: The static assertion actually fired on frv, as reported by the
build bot. Add __packed to fix that.

 lib/vsprintf.c | 41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index ba00eab1e703..dc0beddc036c 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -380,13 +380,13 @@ enum format_type {
 };
 
 struct printf_spec {
-	u8	type;		/* format_type enum */
-	u8	flags;		/* flags to number() */
-	u8	base;		/* number base, 8, 10 or 16 only */
-	u8	qualifier;	/* number qualifier, one of 'hHlLtzZ' */
-	s16	field_width;	/* width of output field */
-	s16	precision;	/* # of digits/chars */
-};
+	unsigned int	type:8;		/* format_type enum */
+	signed int	field_width:24;	/* width of output field */
+	unsigned int	flags:8;	/* flags to number() */
+	unsigned int	base:8;		/* number base, 8, 10 or 16 only */
+	signed int	precision:16;	/* # of digits/chars */
+} __packed;
+extern char __check_printf_spec[1-2*(sizeof(struct printf_spec) != 8)];
 
 static noinline_for_stack
 char *number(char *buf, char *end, unsigned long long num,
@@ -1639,6 +1639,7 @@ static noinline_for_stack
 int format_decode(const char *fmt, struct printf_spec *spec)
 {
 	const char *start = fmt;
+	char qualifier;
 
 	/* we finished early by reading the field width */
 	if (spec->type == FORMAT_TYPE_WIDTH) {
@@ -1721,16 +1722,16 @@ precision:
 
 qualifier:
 	/* get the conversion qualifier */
-	spec->qualifier = -1;
+	qualifier = 0;
 	if (*fmt == 'h' || _tolower(*fmt) == 'l' ||
 	    _tolower(*fmt) == 'z' || *fmt == 't') {
-		spec->qualifier = *fmt++;
-		if (unlikely(spec->qualifier == *fmt)) {
-			if (spec->qualifier == 'l') {
-				spec->qualifier = 'L';
+		qualifier = *fmt++;
+		if (unlikely(qualifier == *fmt)) {
+			if (qualifier == 'l') {
+				qualifier = 'L';
 				++fmt;
-			} else if (spec->qualifier == 'h') {
-				spec->qualifier = 'H';
+			} else if (qualifier == 'h') {
+				qualifier = 'H';
 				++fmt;
 			}
 		}
@@ -1787,19 +1788,19 @@ qualifier:
 		return fmt - start;
 	}
 
-	if (spec->qualifier == 'L')
+	if (qualifier == 'L')
 		spec->type = FORMAT_TYPE_LONG_LONG;
-	else if (spec->qualifier == 'l') {
+	else if (qualifier == 'l') {
 		BUILD_BUG_ON(FORMAT_TYPE_ULONG + SIGN != FORMAT_TYPE_LONG);
 		spec->type = FORMAT_TYPE_ULONG + (spec->flags & SIGN);
-	} else if (_tolower(spec->qualifier) == 'z') {
+	} else if (_tolower(qualifier) == 'z') {
 		spec->type = FORMAT_TYPE_SIZE_T;
-	} else if (spec->qualifier == 't') {
+	} else if (qualifier == 't') {
 		spec->type = FORMAT_TYPE_PTRDIFF;
-	} else if (spec->qualifier == 'H') {
+	} else if (qualifier == 'H') {
 		BUILD_BUG_ON(FORMAT_TYPE_UBYTE + SIGN != FORMAT_TYPE_BYTE);
 		spec->type = FORMAT_TYPE_UBYTE + (spec->flags & SIGN);
-	} else if (spec->qualifier == 'h') {
+	} else if (qualifier == 'h') {
 		BUILD_BUG_ON(FORMAT_TYPE_USHORT + SIGN != FORMAT_TYPE_SHORT);
 		spec->type = FORMAT_TYPE_USHORT + (spec->flags & SIGN);
 	} else {
-- 
2.6.1

Re: [PATCH 04/13] lib/vsprintf.c: expand field_width to 24 bits

[Andrew Morton]

On Tue, 20 Oct 2015 22:30:04 +0200 Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

> I didn't find a BUILD_BUG/compiletime_assertion/... which would work
> outside function context, so for now I just open-coded it.
> 

It comes up occasionally.  It would be better to create one.

> +extern char __check_printf_spec[1-2*(sizeof(struct printf_spec) != 8)];

Maybe something like

/*
 * Description goes here
 */
#define BUILD_BUG_ON_STATIC(unique_id, expr)		\
	typedef char unique_id[1-2*(expr)];

BUILD_BUG_ON_STATIC(__check_printf_spec, sizeof(struct printf_spec) != 8);

("static" seems the wrong term, but what is the correct term for
"outside of functions"?)

(I hope this patchset is still up-to-date)

Re: [PATCH 04/13] lib/vsprintf.c: expand field_width to 24 bits

[Andy Shevchenko]

On Wed, Dec 2, 2015 at 1:38 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Tue, 20 Oct 2015 22:30:04 +0200 Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
>
>> I didn't find a BUILD_BUG/compiletime_assertion/... which would work
>> outside function context, so for now I just open-coded it.
>>
>
> It comes up occasionally.  It would be better to create one.
>
>> +extern char __check_printf_spec[1-2*(sizeof(struct printf_spec) != 8)];
>
> Maybe something like
>
> /*
>  * Description goes here
>  */
> #define BUILD_BUG_ON_STATIC(unique_id, expr)            \
>         typedef char unique_id[1-2*(expr)];

Yeah, it would be really nice to get rid of unique_id, though I have
no idea how to achieve.

>
> BUILD_BUG_ON_STATIC(__check_printf_spec, sizeof(struct printf_spec) != 8);
>
> ("static" seems the wrong term, but what is the correct term for
> "outside of functions"?)

Just _EXT?  Or _VAR?


-- 
With Best Regards,
Andy Shevchenko

[PATCH 05/13] lib/vsprintf.c: help gcc make number() smaller

[Rasmus Villemoes]

One consequence of the reorganization of struct printf_spec to make
field_width 24 bits was that number() gained about 180 bytes. Since
spec is never passed to other functions, we can help gcc make number()
lose most of that extra weight by using local variables for the field
width and precision.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/vsprintf.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 373da7a84382..6d75a3364683 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -399,6 +399,8 @@ char *number(char *buf, char *end, unsigned long long num,
 	int need_pfx = ((spec.flags & SPECIAL) && spec.base != 10);
 	int i;
 	bool is_zero = num == 0LL;
+	int field_width = spec.field_width;
+	int precision = spec.precision;
 
 	/* locase = 0 or 0x20. ORing digits or letters with 'locase'
 	 * produces same digits or (maybe lowercased) letters */
@@ -410,20 +412,20 @@ char *number(char *buf, char *end, unsigned long long num,
 		if ((signed long long)num < 0) {
 			sign = '-';
 			num = -(signed long long)num;
-			spec.field_width--;
+			field_width--;
 		} else if (spec.flags & PLUS) {
 			sign = '+';
-			spec.field_width--;
+			field_width--;
 		} else if (spec.flags & SPACE) {
 			sign = ' ';
-			spec.field_width--;
+			field_width--;
 		}
 	}
 	if (need_pfx) {
 		if (spec.base == 16)
-			spec.field_width -= 2;
+			field_width -= 2;
 		else if (!is_zero)
-			spec.field_width--;
+			field_width--;
 	}
 
 	/* generate full string in tmp[], in reverse order */
@@ -445,12 +447,12 @@ char *number(char *buf, char *end, unsigned long long num,
 	}
 
 	/* printing 100 using %2d gives "100", not "00" */
-	if (i > spec.precision)
-		spec.precision = i;
+	if (i > precision)
+		precision = i;
 	/* leading space padding */
-	spec.field_width -= spec.precision;
+	field_width -= precision;
 	if (!(spec.flags & (ZEROPAD | LEFT))) {
-		while (--spec.field_width >= 0) {
+		while (--field_width >= 0) {
 			if (buf < end)
 				*buf = ' ';
 			++buf;
@@ -479,14 +481,14 @@ char *number(char *buf, char *end, unsigned long long num,
 	if (!(spec.flags & LEFT)) {
 		char c = ' ' + (spec.flags & ZEROPAD);
 		BUILD_BUG_ON(' ' + ZEROPAD != '0');
-		while (--spec.field_width >= 0) {
+		while (--field_width >= 0) {
 			if (buf < end)
 				*buf = c;
 			++buf;
 		}
 	}
 	/* hmm even more zero padding? */
-	while (i <= --spec.precision) {
+	while (i <= --precision) {
 		if (buf < end)
 			*buf = '0';
 		++buf;
@@ -498,7 +500,7 @@ char *number(char *buf, char *end, unsigned long long num,
 		++buf;
 	}
 	/* trailing space padding */
-	while (--spec.field_width >= 0) {
+	while (--field_width >= 0) {
 		if (buf < end)
 			*buf = ' ';
 		++buf;
-- 
2.6.1

[PATCH 06/13] lib/vsprintf.c: warn about too large precisions and field widths

[Rasmus Villemoes]

The field width is overloaded to pass some extra information for
some %p extensions (e.g. #bits for %pb). But we might silently
truncate the passed value when we stash it in struct printf_spec (see
e.g. "lib/vsprintf.c: expand field_width to 24 bits").  Hopefully 23
value bits should now be enough for everybody, but if not, let's make
some noise.

Do the same for the precision. In both cases, clamping seems more
sensible than truncating. A negative precision is the same as 0, so
use that as lower bound. For the field width, the smallest
representable value is actually -(1<<23), but a negative field width
means 'set the LEFT flag and use the absolute value', so we want the
absolute value to fit.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/vsprintf.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 6d75a3364683..3e96cf45c681 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -386,6 +386,8 @@ struct printf_spec {
 	unsigned int	base:8;		/* number base, 8, 10 or 16 only */
 	signed int	precision:16;	/* # of digits/chars */
 };
+#define FIELD_WIDTH_MAX ((1 << 23) - 1)
+#define PRECISION_MAX ((1 << 15) - 1)
 extern char __check_printf_spec[1-2*(sizeof(struct printf_spec) != 8)];
 
 static noinline_for_stack
@@ -1813,6 +1815,24 @@ qualifier:
 	return ++fmt - start;
 }
 
+static inline void
+set_field_width(struct printf_spec *spec, int width)
+{
+	spec->field_width = width;
+	if (WARN_ONCE(spec->field_width != width, "field width %d too large", width)) {
+		spec->field_width = clamp(width, -FIELD_WIDTH_MAX, FIELD_WIDTH_MAX);
+	}
+}
+
+static inline void
+set_precision(struct printf_spec *spec, int prec)
+{
+	spec->precision = prec;
+	if (WARN_ONCE(spec->precision != prec, "precision %d too large", prec)) {
+		spec->precision = clamp(prec, 0, PRECISION_MAX);
+	}
+}
+
 /**
  * vsnprintf - Format a string and place it in a buffer
  * @buf: The buffer to place the result into
@@ -1905,11 +1925,11 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
 		}
 
 		case FORMAT_TYPE_WIDTH:
-			spec.field_width = va_arg(args, int);
+			set_field_width(&spec, va_arg(args, int));
 			break;
 
 		case FORMAT_TYPE_PRECISION:
-			spec.precision = va_arg(args, int);
+			set_precision(&spec, va_arg(args, int));
 			break;
 
 		case FORMAT_TYPE_CHAR: {
-- 
2.6.1

[PATCH 07/13] lib/test_printf.c: don't BUG

[Rasmus Villemoes]

BUG is a completely unnecessarily big hammer, and we're more likely to
get the internal bug reported if we just pr_err() and ensure the test
suite fails.

Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/test_printf.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index c5a666af9ba5..9232a2add28c 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -91,7 +91,12 @@ __test(const char *expect, int elen, const char *fmt, ...)
 	int rand;
 	char *p;
 
-	BUG_ON(elen >= BUF_SIZE);
+	if (elen >= BUF_SIZE) {
+		pr_err("error in test suite: expected output length %d too long. Format was '%s'.\n",
+		       elen, fmt);
+		failed_tests++;
+		return;
+	}
 
 	va_start(ap, fmt);
 
-- 
2.6.1

Re: [PATCH 07/13] lib/test_printf.c: don't BUG

[Kees Cook]

On Wed, Oct 21, 2015 at 5:30 AM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> BUG is a completely unnecessarily big hammer, and we're more likely to
> get the internal bug reported if we just pr_err() and ensure the test
> suite fails.
>
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  lib/test_printf.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index c5a666af9ba5..9232a2add28c 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -91,7 +91,12 @@ __test(const char *expect, int elen, const char *fmt, ...)
>         int rand;
>         char *p;
>
> -       BUG_ON(elen >= BUF_SIZE);
> +       if (elen >= BUF_SIZE) {
> +               pr_err("error in test suite: expected output length %d too long. Format was '%s'.\n",
> +                      elen, fmt);
> +               failed_tests++;
> +               return;
> +       }
>
>         va_start(ap, fmt);
>
> --
> 2.6.1
>



-- 
Kees Cook
Chrome OS Security

[PATCH 08/13] lib/test_printf.c: check for out-of-bound writes

[Rasmus Villemoes]

Add a few padding bytes on either side of the test buffer, and check
that these (and the part of the buffer not used) are untouched by
vsnprintf.

Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/test_printf.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 9232a2add28c..1ce1a1dd8faf 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -16,6 +16,7 @@
 #include <linux/in.h>
 
 #define BUF_SIZE 256
+#define PAD_SIZE 16
 #define FILL_CHAR '$'
 
 #define PTR1 ((void*)0x01234567)
@@ -39,6 +40,7 @@
 static unsigned total_tests __initdata;
 static unsigned failed_tests __initdata;
 static char *test_buffer __initdata;
+static char *alloced_buffer __initdata;
 
 static int __printf(4, 0) __init
 do_test(int bufsize, const char *expect, int elen,
@@ -49,7 +51,7 @@ do_test(int bufsize, const char *expect, int elen,
 
 	total_tests++;
 
-	memset(test_buffer, FILL_CHAR, BUF_SIZE);
+	memset(alloced_buffer, FILL_CHAR, BUF_SIZE + 2*PAD_SIZE);
 	va_copy(aq, ap);
 	ret = vsnprintf(test_buffer, bufsize, fmt, aq);
 	va_end(aq);
@@ -60,8 +62,13 @@ do_test(int bufsize, const char *expect, int elen,
 		return 1;
 	}
 
+	if (memchr_inv(alloced_buffer, FILL_CHAR, PAD_SIZE)) {
+		pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote before buffer\n", bufsize, fmt);
+		return 1;
+	}
+
 	if (!bufsize) {
-		if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE)) {
+		if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE + PAD_SIZE)) {
 			pr_warn("vsnprintf(buf, 0, \"%s\", ...) wrote to buffer\n",
 				fmt);
 			return 1;
@@ -76,6 +83,12 @@ do_test(int bufsize, const char *expect, int elen,
 		return 1;
 	}
 
+	if (memchr_inv(test_buffer + written + 1, FILL_CHAR, BUF_SIZE + PAD_SIZE - (written + 1))) {
+		pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n",
+			bufsize, fmt);
+		return 1;
+	}
+
 	if (memcmp(test_buffer, expect, written)) {
 		pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'\n",
 			bufsize, fmt, test_buffer, written, expect);
@@ -342,16 +355,17 @@ test_pointer(void)
 static int __init
 test_printf_init(void)
 {
-	test_buffer = kmalloc(BUF_SIZE, GFP_KERNEL);
-	if (!test_buffer)
+	alloced_buffer = kmalloc(BUF_SIZE + 2*PAD_SIZE, GFP_KERNEL);
+	if (!alloced_buffer)
 		return -ENOMEM;
+	test_buffer = alloced_buffer + PAD_SIZE;
 
 	test_basic();
 	test_number();
 	test_string();
 	test_pointer();
 
-	kfree(test_buffer);
+	kfree(alloced_buffer);
 
 	if (failed_tests == 0)
 		pr_info("all %u tests passed\n", total_tests);
-- 
2.6.1

Re: [PATCH 08/13] lib/test_printf.c: check for out-of-bound writes

[Kees Cook]

On Wed, Oct 21, 2015 at 5:30 AM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> Add a few padding bytes on either side of the test buffer, and check
> that these (and the part of the buffer not used) are untouched by
> vsnprintf.
>
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

Cool, I like this. :)

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  lib/test_printf.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 9232a2add28c..1ce1a1dd8faf 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -16,6 +16,7 @@
>  #include <linux/in.h>
>
>  #define BUF_SIZE 256
> +#define PAD_SIZE 16
>  #define FILL_CHAR '$'
>
>  #define PTR1 ((void*)0x01234567)
> @@ -39,6 +40,7 @@
>  static unsigned total_tests __initdata;
>  static unsigned failed_tests __initdata;
>  static char *test_buffer __initdata;
> +static char *alloced_buffer __initdata;
>
>  static int __printf(4, 0) __init
>  do_test(int bufsize, const char *expect, int elen,
> @@ -49,7 +51,7 @@ do_test(int bufsize, const char *expect, int elen,
>
>         total_tests++;
>
> -       memset(test_buffer, FILL_CHAR, BUF_SIZE);
> +       memset(alloced_buffer, FILL_CHAR, BUF_SIZE + 2*PAD_SIZE);
>         va_copy(aq, ap);
>         ret = vsnprintf(test_buffer, bufsize, fmt, aq);
>         va_end(aq);
> @@ -60,8 +62,13 @@ do_test(int bufsize, const char *expect, int elen,
>                 return 1;
>         }
>
> +       if (memchr_inv(alloced_buffer, FILL_CHAR, PAD_SIZE)) {
> +               pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote before buffer\n", bufsize, fmt);
> +               return 1;
> +       }
> +
>         if (!bufsize) {
> -               if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE)) {
> +               if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE + PAD_SIZE)) {
>                         pr_warn("vsnprintf(buf, 0, \"%s\", ...) wrote to buffer\n",
>                                 fmt);
>                         return 1;
> @@ -76,6 +83,12 @@ do_test(int bufsize, const char *expect, int elen,
>                 return 1;
>         }
>
> +       if (memchr_inv(test_buffer + written + 1, FILL_CHAR, BUF_SIZE + PAD_SIZE - (written + 1))) {
> +               pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n",
> +                       bufsize, fmt);
> +               return 1;
> +       }
> +
>         if (memcmp(test_buffer, expect, written)) {
>                 pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'\n",
>                         bufsize, fmt, test_buffer, written, expect);
> @@ -342,16 +355,17 @@ test_pointer(void)
>  static int __init
>  test_printf_init(void)
>  {
> -       test_buffer = kmalloc(BUF_SIZE, GFP_KERNEL);
> -       if (!test_buffer)
> +       alloced_buffer = kmalloc(BUF_SIZE + 2*PAD_SIZE, GFP_KERNEL);
> +       if (!alloced_buffer)
>                 return -ENOMEM;
> +       test_buffer = alloced_buffer + PAD_SIZE;
>
>         test_basic();
>         test_number();
>         test_string();
>         test_pointer();
>
> -       kfree(test_buffer);
> +       kfree(alloced_buffer);
>
>         if (failed_tests == 0)
>                 pr_info("all %u tests passed\n", total_tests);
> --
> 2.6.1
>



-- 
Kees Cook
Chrome OS Security

[PATCH 09/13] lib/test_printf.c: add a few string tests

[Rasmus Villemoes]

Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/test_printf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 1ce1a1dd8faf..80ddafb2675d 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -166,6 +166,10 @@ test_string(void)
 	test("", "%s%.0s", "", "123");
 	test("ABCD|abc|123", "%s|%.3s|%.*s", "ABCD", "abcdef", 3, "123456");
 	test("1  |  2|3  |  4|5  ", "%-3s|%3s|%-*s|%*s|%*s", "1", "2", 3, "3", 3, "4", -3, "5");
+	test("1234      ", "%-10.4s", "123456");
+	test("      1234", "%10.4s", "123456");
+	/* Negative precision should be treated as 0. */
+	test("    ", "%4.*s", -5, "123456");
 	/*
 	 * POSIX and C99 say that a missing precision should be
 	 * treated as a precision of 0. However, the kernel's printf
-- 
2.6.1

Re: [PATCH 09/13] lib/test_printf.c: add a few string tests

[Kees Cook]

On Wed, Oct 21, 2015 at 5:30 AM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

I would expect at least a short commit message, with something like
"check string width truncation", but otherwise, I like it. :)

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  lib/test_printf.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 1ce1a1dd8faf..80ddafb2675d 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -166,6 +166,10 @@ test_string(void)
>         test("", "%s%.0s", "", "123");
>         test("ABCD|abc|123", "%s|%.3s|%.*s", "ABCD", "abcdef", 3, "123456");
>         test("1  |  2|3  |  4|5  ", "%-3s|%3s|%-*s|%*s|%*s", "1", "2", 3, "3", 3, "4", -3, "5");
> +       test("1234      ", "%-10.4s", "123456");
> +       test("      1234", "%10.4s", "123456");
> +       /* Negative precision should be treated as 0. */
> +       test("    ", "%4.*s", -5, "123456");
>         /*
>          * POSIX and C99 say that a missing precision should be
>          * treated as a precision of 0. However, the kernel's printf
> --
> 2.6.1
>



-- 
Kees Cook
Chrome OS Security

[PATCH 10/13] lib/test_printf.c: account for kvasprintf tests

[Rasmus Villemoes]

These should also count as performed tests.

Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/test_printf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 80ddafb2675d..8fca007d9bcd 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -127,6 +127,7 @@ __test(const char *expect, int elen, const char *fmt, ...)
 
 	p = kvasprintf(GFP_KERNEL, fmt, ap);
 	if (p) {
+		total_tests++;
 		if (memcmp(p, expect, elen+1)) {
 			pr_warn("kvasprintf(..., \"%s\", ...) returned '%s', expected '%s'\n",
 				fmt, p, expect);
-- 
2.6.1

Re: [PATCH 10/13] lib/test_printf.c: account for kvasprintf tests

[Kees Cook]

On Wed, Oct 21, 2015 at 5:30 AM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> These should also count as performed tests.
>
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  lib/test_printf.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 80ddafb2675d..8fca007d9bcd 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -127,6 +127,7 @@ __test(const char *expect, int elen, const char *fmt, ...)
>
>         p = kvasprintf(GFP_KERNEL, fmt, ap);
>         if (p) {
> +               total_tests++;
>                 if (memcmp(p, expect, elen+1)) {
>                         pr_warn("kvasprintf(..., \"%s\", ...) returned '%s', expected '%s'\n",
>                                 fmt, p, expect);
> --
> 2.6.1
>



-- 
Kees Cook
Chrome OS Security

[PATCH 11/13] lib/test_printf.c: add test for large bitmaps

[Rasmus Villemoes]

Following "lib/vsprintf.c: expand field_width to 24 bits", let's add a
test to see that we now actually support bitmaps with 65536 bits.

Cc: Maurizio Lombardi <mlombard@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/test_printf.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 8fca007d9bcd..19fa411dba78 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -12,6 +12,7 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 
+#include <linux/bitmap.h>
 #include <linux/socket.h>
 #include <linux/in.h>
 
@@ -312,6 +313,20 @@ struct_clk(void)
 }
 
 static void __init
+large_bitmap(void)
+{
+	const int nbits = 1 << 16;
+	unsigned long *bits = kcalloc(BITS_TO_LONGS(nbits), sizeof(long), GFP_KERNEL);
+	if (!bits)
+		return;
+
+	bitmap_set(bits, 1, 20);
+	bitmap_set(bits, 60000, 15);
+	test("1-20,60000-60014", "%*pbl", nbits, bits);
+	kfree(bits);
+}
+
+static void __init
 bitmap(void)
 {
 	DECLARE_BITMAP(bits, 20);
@@ -330,6 +345,8 @@ bitmap(void)
 	bitmap_fill(bits, 20);
 	test("fffff|fffff", "%20pb|%*pb", bits, 20, bits);
 	test("0-19|0-19", "%20pbl|%*pbl", bits, 20, bits);
+
+	large_bitmap();
 }
 
 static void __init
-- 
2.6.1

Re: [PATCH 11/13] lib/test_printf.c: add test for large bitmaps

[Kees Cook]

On Wed, Oct 21, 2015 at 5:30 AM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> Following "lib/vsprintf.c: expand field_width to 24 bits", let's add a
> test to see that we now actually support bitmaps with 65536 bits.
>
> Cc: Maurizio Lombardi <mlombard@redhat.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Tejun Heo <tj@kernel.org>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  lib/test_printf.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 8fca007d9bcd..19fa411dba78 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -12,6 +12,7 @@
>  #include <linux/slab.h>
>  #include <linux/string.h>
>
> +#include <linux/bitmap.h>
>  #include <linux/socket.h>
>  #include <linux/in.h>
>
> @@ -312,6 +313,20 @@ struct_clk(void)
>  }
>
>  static void __init
> +large_bitmap(void)
> +{
> +       const int nbits = 1 << 16;
> +       unsigned long *bits = kcalloc(BITS_TO_LONGS(nbits), sizeof(long), GFP_KERNEL);
> +       if (!bits)
> +               return;
> +
> +       bitmap_set(bits, 1, 20);
> +       bitmap_set(bits, 60000, 15);
> +       test("1-20,60000-60014", "%*pbl", nbits, bits);
> +       kfree(bits);
> +}
> +
> +static void __init
>  bitmap(void)
>  {
>         DECLARE_BITMAP(bits, 20);
> @@ -330,6 +345,8 @@ bitmap(void)
>         bitmap_fill(bits, 20);
>         test("fffff|fffff", "%20pb|%*pb", bits, 20, bits);
>         test("0-19|0-19", "%20pbl|%*pbl", bits, 20, bits);
> +
> +       large_bitmap();
>  }
>
>  static void __init
> --
> 2.6.1
>



-- 
Kees Cook
Chrome OS Security

[PATCH 12/13] lib/test_printf.c: test dentry printing

[Rasmus Villemoes]

Cc: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
Al, I'd appreciate it if you'd take a look and see that the printed
strings are actually as expected.

 lib/test_printf.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 19fa411dba78..8ddaad27d3b4 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -13,6 +13,7 @@
 #include <linux/string.h>
 
 #include <linux/bitmap.h>
+#include <linux/dcache.h>
 #include <linux/socket.h>
 #include <linux/in.h>
 
@@ -297,9 +298,35 @@ uuid(void)
 	test("03020100-0504-0706-0809-0A0B0C0D0E0F", "%pUL", uuid);
 }
 
+static struct dentry test_dentry[4] __initdata = {
+	{ .d_parent = &test_dentry[0],
+	  .d_name = { .len = 3, .name = test_dentry[0].d_iname },
+	  .d_iname = "foo" },
+	{ .d_parent = &test_dentry[0],
+	  .d_name = { .len = 5, .name = test_dentry[1].d_iname },
+	  .d_iname = "bravo" },
+	{ .d_parent = &test_dentry[1],
+	  .d_name = { .len = 4, .name = test_dentry[2].d_iname },
+	  .d_iname = "alfa" },
+	{ .d_parent = &test_dentry[2],
+	  .d_name = { .len = 5, .name = test_dentry[3].d_iname },
+	  .d_iname = "romeo" },
+};
+
 static void __init
 dentry(void)
 {
+	test("foo", "%pd", &test_dentry[0]);
+	test("foo", "%pd2", &test_dentry[0]);
+
+	test("romeo", "%pd", &test_dentry[3]);
+	test("alfa/romeo", "%pd2", &test_dentry[3]);
+	test("bravo/alfa/romeo", "%pd3", &test_dentry[3]);
+	test("/bravo/alfa/romeo", "%pd4", &test_dentry[3]);
+	test("/bravo/alfa", "%pd4", &test_dentry[2]);
+
+	test("bravo/alfa  |bravo/alfa  ", "%-12pd2|%*pd2", &test_dentry[2], -12, &test_dentry[2]);
+	test("  bravo/alfa|  bravo/alfa", "%12pd2|%*pd2", &test_dentry[2], 12, &test_dentry[2]);
 }
 
 static void __init
-- 
2.6.1

[PATCH 13/13] lib/kasprintf.c: add sanity check to kvasprintf

[Rasmus Villemoes]

kasprintf relies on being able to replay the formatting and getting
the same result (in particular, the same length). This will almost
always work, but it is possible that the object pointed to by a %s or
%p argument changed under us (so we might get truncated output). Add a
somewhat paranoid sanity check and let's see if it ever triggers.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/kasprintf.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/kasprintf.c b/lib/kasprintf.c
index 32f12150fc4f..a30bf3042020 100644
--- a/lib/kasprintf.c
+++ b/lib/kasprintf.c
@@ -13,19 +13,21 @@
 /* Simplified asprintf. */
 char *kvasprintf(gfp_t gfp, const char *fmt, va_list ap)
 {
-	unsigned int len;
+	unsigned int first, second;
 	char *p;
 	va_list aq;
 
 	va_copy(aq, ap);
-	len = vsnprintf(NULL, 0, fmt, aq);
+	first = vsnprintf(NULL, 0, fmt, aq);
 	va_end(aq);
 
-	p = kmalloc_track_caller(len+1, gfp);
+	p = kmalloc_track_caller(first+1, gfp);
 	if (!p)
 		return NULL;
 
-	vsnprintf(p, len+1, fmt, ap);
+	second = vsnprintf(p, first+1, fmt, ap);
+	WARN(first != second, "different return values (%u and %u) from vsnprintf(\"%s\", ...)",
+	     first, second, fmt);
 
 	return p;
 }
-- 
2.6.1