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

From: Thierry Escande <thierry.escande@linaro.org>
To: Petr Mladek <pmladek@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	David Miller <davem@davemloft.net>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	"Tobin C . Harding" <me@tobin.cc>,
	linux-kernel@vger.kernel.org,
	Andy Shevchenko <andy.shevchenko@gmail.com>
Subject: Re: [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests
Date: Fri, 8 Jun 2018 11:07:11 +0200	[thread overview]
Message-ID: <0fa30e13-c16f-95d8-d1ef-fd45e7a99888@linaro.org> (raw)
In-Reply-To: <20180608080437.n2b5nv73d3kp6yhb@pathway.suse.cz>

On 08/06/2018 10:04, Petr Mladek wrote:
> On Thu 2018-06-07 20:47:25, Thierry Escande wrote:
>> Hi Petr,
>>
>> On 07/06/2018 14:24, Petr Mladek wrote:
>>> On Mon 2018-06-04 13:37:08, Thierry Escande 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.
>>>> This patch adds a call to wait_for_random_bytes() before plain 'p' tests
>>>> to make sure the crng is initialized.
>>>
>>> Hmm, my system did not boot with this patch and
>>> CONFIG_TEST_PRINTF=y
>>>
>>> I guess that there was not enough entropy. It is basically a deadlock.
>>> The build-in module init calls are called when SMP is enabled but
>>> there is no real activity and the init calls are called sequentially.
>>> Therefore there is nothing that could produce some entropy, ...
>>>
>>> I suggest to skip the test when crng is not ready.
>>
>> Thanks for pointing this out.
>>
>> The issue is that the crng_ready() is not available outside of random.c.
>> I'll leave the call to wait_for_random_bytes() if test_printf is compiled as
>> a module and use add_random_ready_callback() to check if crng is initialized
>> otherwise. Does that sound ok to you?
> 
> Great, I was not aware of add_random_ready_callback().
> 
> I suggest to make the decision in test_printf_init()
> and run all tests either as the callback or immediately.
> 

The run_kselftest.sh script relies on the error code returned by 
modprobe to check if the test is passed or not. So the init function has 
to return the test results and we cannot defer the test execution in the 
random ready callback if the crng is not yet initialized at __init.

My idea was to use add_random_ready_callback() as a non blocking test 
when compiled built-in as it returns -EALREADY if crng is already 
initialized.

But as I type I realize it's not necessary. I will simply enclose the 
call to wait_for_random_bytes() by #if IS_MODULE() #endif so it gets 
called only if built as a module, which is how run_kselftest.sh wants 
it... If test_printf is compiled built-in and the crng is not yet 
initialized the test will fail anyway so there is no need to add an 
extra check.

Regards,
Thierry