linux-kernel - [RFC PATCH] vsprintf: add flag ZEROPAD handling before crng is ready

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <1516952396-7423-1-git-send-email-shunyong.yang@hxt-semitech.com>
Date:   Fri, 26 Jan 2018 15:39:56 +0800
From:   Yang Shunyong <shunyong.yang@...-semitech.com>
To:     <me@...in.cc>
CC:     <akpm@...ux-foundation.org>, <joe@...ches.com>,
        <andriy.shevchenko@...ux.intel.com>,
        <pantelis.antoniou@...sulko.com>, <mchehab@...nel.org>,
        <adobriyan@...il.com>, <linux-kernel@...r.kernel.org>,
        <yu.zheng@...-semitech.com>,
        Yang Shunyong <shunyong.yang@...-semitech.com>
Subject: [RFC PATCH] vsprintf: add flag ZEROPAD handling before crng is ready

Before crng is ready, output of "%p" composes of "(ptrval)" and
left padding spaces for alignment as no random address can be
generated. This seems a little strange sometimes.
For example, when irq domain names are built with "%p", the nodes
under /sys/kernel/debug/irq/domains like this,

[root@y irq]# ls domains/
default                   irqchip@        (ptrval)-2
irqchip@        (ptrval)-4  \_SB_.TCS0.QIC1  \_SB_.TCS0.QIC3
irqchip@        (ptrval)  irqchip@        (ptrval)-3
\_SB_.TCS0.QIC0             \_SB_.TCS0.QIC2

The name "irqchip@        (ptrval)-2" is not so readable in console
output.
This patch adds ZEROPAD handling in widen_string() and move_right().
When ZEROPAD is set in spec, it will use '0' for padding. If not
set, it will use ' '.
This patch also sets ZEROPAD in ptr_to_id() before crgn is ready.
Following is the output after applying the patch,

[root@y irq]# ls domains/
default                   irqchip@...00000(ptrval)-2
irqchip@...00000(ptrval)-4  \_SB_.TCS0.QIC1  \_SB_.TCS0.QIC3
irqchip@...00000(ptrval)  irqchip@...00000(ptrval)-3  \_SB_.TCS0.QIC0
\_SB_.TCS0.QIC2

I am not sure whether crng can get enough random data at very early
stage of system startup (eg. before irq system in the case above)
and the effort to port current random driver to work at that time.
So, this issue may exist some time.
I use '0' for padding in this patch. This should be ok because output
of "%pK" is all '0's when kptr_restrict = 2. I don't know whether
other character, such as 'x', may be more preferable.

Signed-off-by: Yang Shunyong <shunyong.yang@...-semitech.com>
---
 lib/vsprintf.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 01c3957b2de6..e0b6e1edae31 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -535,14 +535,18 @@ char *special_hex_number(char *buf, char *end, unsigned long long num, int size)
 	return number(buf, end, num, spec);
 }
 
-static void move_right(char *buf, char *end, unsigned len, unsigned spaces)
+static void move_right(char *buf, char *end, unsigned int len,
+		       unsigned int spaces, struct printf_spec spec)
 {
 	size_t size;
+	char pad;
+
+	pad = (spec.flags & ZEROPAD) ? '0' : ' ';
 	if (buf >= end)	/* nowhere to put anything */
 		return;
 	size = end - buf;
 	if (size <= spaces) {
-		memset(buf, ' ', size);
+		memset(buf, pad, size);
 		return;
 	}
 	if (len) {
@@ -550,7 +554,7 @@ static void move_right(char *buf, char *end, unsigned len, unsigned spaces)
 			len = size - spaces;
 		memmove(buf + spaces, buf, len);
 	}
-	memset(buf, ' ', spaces);
+	memset(buf, pad, spaces);
 }
 
 /*
@@ -565,18 +569,21 @@ static void move_right(char *buf, char *end, unsigned len, unsigned spaces)
 char *widen_string(char *buf, int n, char *end, struct printf_spec spec)
 {
 	unsigned spaces;
+	char pad;
 
 	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);
+		move_right(buf - n, end, n, spaces, spec);
 		return buf + spaces;
 	}
+
+	pad = (spec.flags & ZEROPAD) ? '0' : ' ';
 	while (spaces--) {
 		if (buf < end)
-			*buf = ' ';
+			*buf = pad;
 		++buf;
 	}
 	return buf;
@@ -1702,6 +1709,8 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
 
 	if (unlikely(!have_filled_random_ptr_key)) {
 		spec.field_width = default_width;
+		spec.flags |= ZEROPAD;
+
 		/* string length must be less than default_width */
 		return string(buf, end, "(ptrval)", spec);
 	}
-- 
1.8.3.1