[PATCH v2] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests

[PATCH v2] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests

[Thierry Escande]

If the test_printf module is loaded before the crng is initialized, the
plain 'p' tests will fail because the printed address will not be hashed
and the buffer will contain "(ptrval)" instead.
Since we cannot wait for the crng to be initialized for an undefined
time, both plain 'p' tests now accept the string "(ptrval)" as a valid
result and print a warning message.

Signed-off-by: Thierry Escande <thierry.escande@linaro.org>

---

Change in v2:
- Remove wait_for_random_bytes() usage
- Removed Acked-by from Tobin as the proposed solution is not the same
  anymore.

 lib/test_printf.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 71ebfa43ad05..a625e3749566 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -206,6 +206,7 @@ test_string(void)
 #define PTR_WIDTH 16
 #define PTR ((void *)0xffff0123456789ab)
 #define PTR_STR "ffff0123456789ab"
+#define PTR_VAL_NO_CRNG "        (ptrval)"
 #define ZEROS "00000000"	/* hex 32 zero bits */
 
 static int __init
@@ -216,7 +217,16 @@ plain_format(void)
 
 	nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);
 
-	if (nchars != PTR_WIDTH || strncmp(buf, ZEROS, strlen(ZEROS)) != 0)
+	if (nchars != PTR_WIDTH)
+		return -1;
+
+	if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
+		pr_warn("crng possibly not yet initialized. plain 'p' buffer contains \"%s\"",
+			PTR_VAL_NO_CRNG);
+		return 0;
+	}
+
+	if (strncmp(buf, ZEROS, strlen(ZEROS)) != 0)
 		return -1;
 
 	return 0;
@@ -227,6 +237,7 @@ plain_format(void)
 #define PTR_WIDTH 8
 #define PTR ((void *)0x456789ab)
 #define PTR_STR "456789ab"
+#define PTR_VAL_NO_CRNG "(ptrval)"
 
 static int __init
 plain_format(void)
@@ -245,7 +256,16 @@ plain_hash(void)
 
 	nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);
 
-	if (nchars != PTR_WIDTH || strncmp(buf, PTR_STR, PTR_WIDTH) == 0)
+	if (nchars != PTR_WIDTH)
+		return -1;
+
+	if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
+		pr_warn("crng possibly not yet initialized. plain 'p' buffer contains \"%s\"",
+			PTR_VAL_NO_CRNG);
+		return 0;
+	}
+
+	if (strncmp(buf, PTR_STR, PTR_WIDTH) == 0)
 		return -1;
 
 	return 0;
-- 
2.14.1

Re: [PATCH v2] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests

[Andy Shevchenko]

On Wed, Jun 13, 2018 at 12:29 PM, Thierry Escande
<thierry.escande@linaro.org> wrote:
> If the test_printf module is loaded before the crng is initialized, the
> plain 'p' tests will fail because the printed address will not be hashed
> and the buffer will contain "(ptrval)" instead.
> Since we cannot wait for the crng to be initialized for an undefined
> time, both plain 'p' tests now accept the string "(ptrval)" as a valid
> result and print a warning message.

There are two possibilities:
 1. (ptrval) for 32-bit case
 2. (____ptrval____) for 64-bit case.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests

[Thierry Escande]

On 13/06/2018 13:22, Andy Shevchenko wrote:
> On Wed, Jun 13, 2018 at 12:29 PM, Thierry Escande
> <thierry.escande@linaro.org> wrote:
>> If the test_printf module is loaded before the crng is initialized, the
>> plain 'p' tests will fail because the printed address will not be hashed
>> and the buffer will contain "(ptrval)" instead.
>> Since we cannot wait for the crng to be initialized for an undefined
>> time, both plain 'p' tests now accept the string "(ptrval)" as a valid
>> result and print a warning message.
> 
> There are two possibilities:
>   1. (ptrval) for 32-bit case
>   2. (____ptrval____) for 64-bit case.

 From lib/vsprintf.c, ptr_to_id() puts "(ptrval)" into the buffer, then 
it gets left-padded with spaces by widen_string().

Regards,
Thierry

Re: [PATCH v2] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests

[Andy Shevchenko]

On Wed, Jun 13, 2018 at 5:59 PM, Thierry Escande
<thierry.escande@linaro.org> wrote:
> On 13/06/2018 13:22, Andy Shevchenko wrote:
>>
>> On Wed, Jun 13, 2018 at 12:29 PM, Thierry Escande
>> <thierry.escande@linaro.org> wrote:
>>>
>>> If the test_printf module is loaded before the crng is initialized, the
>>> plain 'p' tests will fail because the printed address will not be hashed
>>> and the buffer will contain "(ptrval)" instead.
>>> Since we cannot wait for the crng to be initialized for an undefined
>>> time, both plain 'p' tests now accept the string "(ptrval)" as a valid
>>> result and print a warning message.
>>
>>
>> There are two possibilities:
>>   1. (ptrval) for 32-bit case
>>   2. (____ptrval____) for 64-bit case.
>
>
> From lib/vsprintf.c, ptr_to_id() puts "(ptrval)" into the buffer, then it
> gets left-padded with spaces by widen_string().

Which kernel version you are trying to fix?

What I see for a long time in linux-next:

static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
{
        const char *str = sizeof(ptr) == 8 ? "(____ptrval____)" : "(ptrval)";
...

brought by the commit 91efafb1dd8f ("lib/vsprintf: Replace space with
'_' before crng is ready").

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests

[Thierry Escande]

On 13/06/2018 18:30, Andy Shevchenko wrote:
> On Wed, Jun 13, 2018 at 5:59 PM, Thierry Escande
> <thierry.escande@linaro.org> wrote:
>> On 13/06/2018 13:22, Andy Shevchenko wrote:
>>>
>>> On Wed, Jun 13, 2018 at 12:29 PM, Thierry Escande
>>> <thierry.escande@linaro.org> wrote:
>>>>
>>>> If the test_printf module is loaded before the crng is initialized, the
>>>> plain 'p' tests will fail because the printed address will not be hashed
>>>> and the buffer will contain "(ptrval)" instead.
>>>> Since we cannot wait for the crng to be initialized for an undefined
>>>> time, both plain 'p' tests now accept the string "(ptrval)" as a valid
>>>> result and print a warning message.
>>>
>>>
>>> There are two possibilities:
>>>    1. (ptrval) for 32-bit case
>>>    2. (____ptrval____) for 64-bit case.
>>
>>
>>  From lib/vsprintf.c, ptr_to_id() puts "(ptrval)" into the buffer, then it
>> gets left-padded with spaces by widen_string().
> 
> Which kernel version you are trying to fix?
> 
> What I see for a long time in linux-next:
> 
> static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
> {
>          const char *str = sizeof(ptr) == 8 ? "(____ptrval____)" : "(ptrval)";
> ...
> 
> brought by the commit 91efafb1dd8f ("lib/vsprintf: Replace space with
> '_' before crng is ready").

I missed that one as it's not in v4.17. My bad.

Regards,
Thierry