All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests
@ 2018-06-04 11:37 Thierry Escande
  2018-06-05 12:43 ` Andy Shevchenko
  2018-06-07 12:24 ` Petr Mladek
  0 siblings, 2 replies; 14+ messages in thread
From: Thierry Escande @ 2018-06-04 11:37 UTC (permalink / raw)
  To: Andrew Morton, David Miller
  Cc: Rasmus Villemoes, Tobin C . Harding, linux-kernel

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.

Signed-off-by: Thierry Escande <thierry.escande@linaro.org>
Acked-by: Tobin C. Harding <me@tobin.cc>
---
 lib/test_printf.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 71ebfa43ad05..839be9385a8a 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -260,6 +260,13 @@ plain(void)
 {
 	int err;
 
+	/*
+	 * Make sure crng is ready. Otherwise we get "(ptrval)" instead
+	 * of a hashed address when printing '%p' in plain_hash() and
+	 * plain_format().
+	 */
+	wait_for_random_bytes();
+
 	err = plain_hash();
 	if (err) {
 		pr_warn("plain 'p' does not appear to be hashed\n");
-- 
2.14.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests
  2018-06-04 11:37 [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests Thierry Escande
@ 2018-06-05 12:43 ` Andy Shevchenko
  2018-06-07 12:24 ` Petr Mladek
  1 sibling, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2018-06-05 12:43 UTC (permalink / raw)
  To: Thierry Escande, Petr Mladek
  Cc: Andrew Morton, David Miller, Rasmus Villemoes, Tobin C . Harding,
	Linux Kernel Mailing List

+Cc: Petr. I suppose test_printf is going through his tree as well as
vsnprintf itself. At least it logically makes sense.

On Mon, Jun 4, 2018 at 2:37 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.
> This patch adds a call to wait_for_random_bytes() before plain 'p' tests
> to make sure the crng is initialized.
>
> Signed-off-by: Thierry Escande <thierry.escande@linaro.org>
> Acked-by: Tobin C. Harding <me@tobin.cc>
> ---
>  lib/test_printf.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 71ebfa43ad05..839be9385a8a 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -260,6 +260,13 @@ plain(void)
>  {
>         int err;
>
> +       /*
> +        * Make sure crng is ready. Otherwise we get "(ptrval)" instead
> +        * of a hashed address when printing '%p' in plain_hash() and
> +        * plain_format().
> +        */
> +       wait_for_random_bytes();
> +
>         err = plain_hash();
>         if (err) {
>                 pr_warn("plain 'p' does not appear to be hashed\n");
> --
> 2.14.1
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests
  2018-06-04 11:37 [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests Thierry Escande
  2018-06-05 12:43 ` Andy Shevchenko
@ 2018-06-07 12:24 ` Petr Mladek
  2018-06-07 18:47   ` Thierry Escande
  2018-06-22 20:53   ` Steven Rostedt
  1 sibling, 2 replies; 14+ messages in thread
From: Petr Mladek @ 2018-06-07 12:24 UTC (permalink / raw)
  To: Thierry Escande
  Cc: Andrew Morton, David Miller, Rasmus Villemoes, Tobin C . Harding,
	linux-kernel, Andy Shevchenko

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.

Best Regards,
Petr

> Signed-off-by: Thierry Escande <thierry.escande@linaro.org>
> Acked-by: Tobin C. Harding <me@tobin.cc>
> ---
>  lib/test_printf.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 71ebfa43ad05..839be9385a8a 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -260,6 +260,13 @@ plain(void)
>  {
>  	int err;
>  
> +	/*
> +	 * Make sure crng is ready. Otherwise we get "(ptrval)" instead
> +	 * of a hashed address when printing '%p' in plain_hash() and
> +	 * plain_format().
> +	 */
> +	wait_for_random_bytes();
> +
>  	err = plain_hash();
>  	if (err) {
>  		pr_warn("plain 'p' does not appear to be hashed\n");
> -- 
> 2.14.1
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests
  2018-06-07 12:24 ` Petr Mladek
@ 2018-06-07 18:47   ` Thierry Escande
  2018-06-08  8:04     ` Petr Mladek
  2018-06-22 20:53   ` Steven Rostedt
  1 sibling, 1 reply; 14+ messages in thread
From: Thierry Escande @ 2018-06-07 18:47 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Andrew Morton, David Miller, Rasmus Villemoes, Tobin C . Harding,
	linux-kernel, Andy Shevchenko

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?

Regards,
Thierry

> 
> Best Regards,
> Petr
> 
>> Signed-off-by: Thierry Escande <thierry.escande@linaro.org>
>> Acked-by: Tobin C. Harding <me@tobin.cc>
>> ---
>>   lib/test_printf.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/lib/test_printf.c b/lib/test_printf.c
>> index 71ebfa43ad05..839be9385a8a 100644
>> --- a/lib/test_printf.c
>> +++ b/lib/test_printf.c
>> @@ -260,6 +260,13 @@ plain(void)
>>   {
>>   	int err;
>>   
>> +	/*
>> +	 * Make sure crng is ready. Otherwise we get "(ptrval)" instead
>> +	 * of a hashed address when printing '%p' in plain_hash() and
>> +	 * plain_format().
>> +	 */
>> +	wait_for_random_bytes();
>> +
>>   	err = plain_hash();
>>   	if (err) {
>>   		pr_warn("plain 'p' does not appear to be hashed\n");
>> -- 
>> 2.14.1
>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests
  2018-06-07 18:47   ` Thierry Escande
@ 2018-06-08  8:04     ` Petr Mladek
  2018-06-08  9:07       ` Thierry Escande
  0 siblings, 1 reply; 14+ messages in thread
From: Petr Mladek @ 2018-06-08  8:04 UTC (permalink / raw)
  To: Thierry Escande
  Cc: Andrew Morton, David Miller, Rasmus Villemoes, Tobin C . Harding,
	linux-kernel, Andy Shevchenko

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.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests
  2018-06-08  8:04     ` Petr Mladek
@ 2018-06-08  9:07       ` Thierry Escande
  2018-06-08  9:46         ` Andy Shevchenko
  0 siblings, 1 reply; 14+ messages in thread
From: Thierry Escande @ 2018-06-08  9:07 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Andrew Morton, David Miller, Rasmus Villemoes, Tobin C . Harding,
	linux-kernel, Andy Shevchenko



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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests
  2018-06-08  9:07       ` Thierry Escande
@ 2018-06-08  9:46         ` Andy Shevchenko
  2018-06-08 10:32           ` Thierry Escande
  0 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2018-06-08  9:46 UTC (permalink / raw)
  To: Thierry Escande
  Cc: Petr Mladek, Andrew Morton, David Miller, Rasmus Villemoes,
	Tobin C . Harding, Linux Kernel Mailing List

On Fri, Jun 8, 2018 at 12:07 PM, Thierry Escande
<thierry.escande@linaro.org> wrote:

> 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.

Unfortunately I can't support this as is.
We have environments where crng will be ready minutes after the boot.
It's unacceptable.

So, we need to have means to not delay test for so long.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests
  2018-06-08  9:46         ` Andy Shevchenko
@ 2018-06-08 10:32           ` Thierry Escande
  2018-06-08 11:22             ` Petr Mladek
  0 siblings, 1 reply; 14+ messages in thread
From: Thierry Escande @ 2018-06-08 10:32 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Petr Mladek, Andrew Morton, David Miller, Rasmus Villemoes,
	Tobin C . Harding, Linux Kernel Mailing List

On 08/06/2018 11:46, Andy Shevchenko wrote:
> On Fri, Jun 8, 2018 at 12:07 PM, Thierry Escande
> <thierry.escande@linaro.org> wrote:
> 
>> 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.
> 
> Unfortunately I can't support this as is.
> We have environments where crng will be ready minutes after the boot.
> It's unacceptable.
> 
> So, we need to have means to not delay test for so long.

I agree we can't delay test execution for too long. In my case the crng 
is ready only a few seconds after the boot. So we may just skip this 
plain 'p' printf test if crng is not ready then.

Regards,
Thierry

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests
  2018-06-08 10:32           ` Thierry Escande
@ 2018-06-08 11:22             ` Petr Mladek
  2018-06-08 11:28               ` Thierry Escande
  0 siblings, 1 reply; 14+ messages in thread
From: Petr Mladek @ 2018-06-08 11:22 UTC (permalink / raw)
  To: Thierry Escande
  Cc: Andy Shevchenko, Andrew Morton, David Miller, Rasmus Villemoes,
	Tobin C . Harding, Linux Kernel Mailing List

On Fri 2018-06-08 12:32:33, Thierry Escande wrote:
> On 08/06/2018 11:46, Andy Shevchenko wrote:
> > On Fri, Jun 8, 2018 at 12:07 PM, Thierry Escande
> > <thierry.escande@linaro.org> wrote:
> > 
> > > 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.
> > 
> > Unfortunately I can't support this as is.
> > We have environments where crng will be ready minutes after the boot.
> > It's unacceptable.
> > 
> > So, we need to have means to not delay test for so long.
> 
> I agree we can't delay test execution for too long. In my case the crng is
> ready only a few seconds after the boot. So we may just skip this plain 'p'
> printf test if crng is not ready then.

Alternative solution would be to accept
const char *str = sizeof(ptr) == 8 ? "(____ptrval____)" : "(ptrval)";
as a valid result. It would make sense to print some warning in that case.

In each case, it would look ugly to use add_random_ready_callback()
wihtout passing a callback. If you really needed to check crng_ready(),
it would be better to make it public.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests
  2018-06-08 11:22             ` Petr Mladek
@ 2018-06-08 11:28               ` Thierry Escande
  0 siblings, 0 replies; 14+ messages in thread
From: Thierry Escande @ 2018-06-08 11:28 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Andy Shevchenko, Andrew Morton, David Miller, Rasmus Villemoes,
	Tobin C . Harding, Linux Kernel Mailing List



On 08/06/2018 13:22, Petr Mladek wrote:
> On Fri 2018-06-08 12:32:33, Thierry Escande wrote:
>> On 08/06/2018 11:46, Andy Shevchenko wrote:
>>> On Fri, Jun 8, 2018 at 12:07 PM, Thierry Escande
>>> <thierry.escande@linaro.org> wrote:
>>>
>>>> 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.
>>>
>>> Unfortunately I can't support this as is.
>>> We have environments where crng will be ready minutes after the boot.
>>> It's unacceptable.
>>>
>>> So, we need to have means to not delay test for so long.
>>
>> I agree we can't delay test execution for too long. In my case the crng is
>> ready only a few seconds after the boot. So we may just skip this plain 'p'
>> printf test if crng is not ready then.
> 
> Alternative solution would be to accept
> const char *str = sizeof(ptr) == 8 ? "(____ptrval____)" : "(ptrval)";
> as a valid result. It would make sense to print some warning in that case.
> 
> In each case, it would look ugly to use add_random_ready_callback()
> wihtout passing a callback. If you really needed to check crng_ready(),
> it would be better to make it public.

Agree but even with crng_ready() public we would have to block the test 
until it's ready which is not good either.

Accepting "(ptrval)" as a valid result seems the least bad alternative...

Regards,
Thierry

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests
  2018-06-07 12:24 ` Petr Mladek
  2018-06-07 18:47   ` Thierry Escande
@ 2018-06-22 20:53   ` Steven Rostedt
  2018-06-22 21:50     ` Thierry Escande
  1 sibling, 1 reply; 14+ messages in thread
From: Steven Rostedt @ 2018-06-22 20:53 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Thierry Escande, Andrew Morton, David Miller, Rasmus Villemoes,
	Tobin C . Harding, linux-kernel, Andy Shevchenko, Linus Torvalds

On Thu, Jun 07, 2018 at 02:24:34PM +0200, 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

And neither does my test box. It killed my tests I was running, as one of the
configs I test has this set.

It appears that Andrew pulled it in and sent it to Linus, as it is in
4.18-rc1, and I need to now revert this patch to make my tests work.

-- Steve


> 
> 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.
> 
> Best Regards,
> Petr
> 
> > Signed-off-by: Thierry Escande <thierry.escande@linaro.org>
> > Acked-by: Tobin C. Harding <me@tobin.cc>
> > ---
> >  lib/test_printf.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/lib/test_printf.c b/lib/test_printf.c
> > index 71ebfa43ad05..839be9385a8a 100644
> > --- a/lib/test_printf.c
> > +++ b/lib/test_printf.c
> > @@ -260,6 +260,13 @@ plain(void)
> >  {
> >  	int err;
> >  
> > +	/*
> > +	 * Make sure crng is ready. Otherwise we get "(ptrval)" instead
> > +	 * of a hashed address when printing '%p' in plain_hash() and
> > +	 * plain_format().
> > +	 */
> > +	wait_for_random_bytes();
> > +
> >  	err = plain_hash();
> >  	if (err) {
> >  		pr_warn("plain 'p' does not appear to be hashed\n");
> > -- 
> > 2.14.1
> > 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests
  2018-06-22 20:53   ` Steven Rostedt
@ 2018-06-22 21:50     ` Thierry Escande
  2018-06-25  7:50       ` Petr Mladek
  0 siblings, 1 reply; 14+ messages in thread
From: Thierry Escande @ 2018-06-22 21:50 UTC (permalink / raw)
  To: Steven Rostedt, Petr Mladek
  Cc: Andrew Morton, David Miller, Rasmus Villemoes, Tobin C . Harding,
	linux-kernel, Andy Shevchenko, Linus Torvalds

On 22/06/2018 22:53, Steven Rostedt wrote:
> On Thu, Jun 07, 2018 at 02:24:34PM +0200, 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
> 
> And neither does my test box. It killed my tests I was running, as one of the
> configs I test has this set.
> 
> It appears that Andrew pulled it in and sent it to Linus, as it is in
> 4.18-rc1, and I need to now revert this patch to make my tests work.

This patch has been superseded with a v2 and a v3 pushed into Petr 
printk.git tree 
(https://git.kernel.org/pub/scm/linux/kernel/git/pmladek/printk.git/commit/?h=for-4.19&id=ce041c43f22298485122bab15c14d062383fbc67). 
Sorry for the mess...

Regards,
Thierry

> 
> -- Steve
> 
> 
>>
>> 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.
>>
>> Best Regards,
>> Petr
>>
>>> Signed-off-by: Thierry Escande <thierry.escande@linaro.org>
>>> Acked-by: Tobin C. Harding <me@tobin.cc>
>>> ---
>>>   lib/test_printf.c | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/lib/test_printf.c b/lib/test_printf.c
>>> index 71ebfa43ad05..839be9385a8a 100644
>>> --- a/lib/test_printf.c
>>> +++ b/lib/test_printf.c
>>> @@ -260,6 +260,13 @@ plain(void)
>>>   {
>>>   	int err;
>>>   
>>> +	/*
>>> +	 * Make sure crng is ready. Otherwise we get "(ptrval)" instead
>>> +	 * of a hashed address when printing '%p' in plain_hash() and
>>> +	 * plain_format().
>>> +	 */
>>> +	wait_for_random_bytes();
>>> +
>>>   	err = plain_hash();
>>>   	if (err) {
>>>   		pr_warn("plain 'p' does not appear to be hashed\n");
>>> -- 
>>> 2.14.1
>>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests
  2018-06-22 21:50     ` Thierry Escande
@ 2018-06-25  7:50       ` Petr Mladek
  2018-06-25 12:04         ` Petr Mladek
  0 siblings, 1 reply; 14+ messages in thread
From: Petr Mladek @ 2018-06-25  7:50 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Steven Rostedt, Thierry Escande, David Miller, Rasmus Villemoes,
	Tobin C . Harding, linux-kernel, Andy Shevchenko, Linus Torvalds

On Fri 2018-06-22 23:50:20, Thierry Escande wrote:
> On 22/06/2018 22:53, Steven Rostedt wrote:
> > On Thu, Jun 07, 2018 at 02:24:34PM +0200, 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
> > 
> > And neither does my test box. It killed my tests I was running, as one of the
> > configs I test has this set.
> > 
> > It appears that Andrew pulled it in and sent it to Linus, as it is in
> > 4.18-rc1, and I need to now revert this patch to make my tests work.
> 
> This patch has been superseded with a v2 and a v3 pushed into Petr
> printk.git tree (https://git.kernel.org/pub/scm/linux/kernel/git/pmladek/printk.git/commit/?h=for-4.19&id=ce041c43f22298485122bab15c14d062383fbc67).
> Sorry for the mess...

Andrew,

should I send the revert and the better fix to Linus or would you like
to do so?

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests
  2018-06-25  7:50       ` Petr Mladek
@ 2018-06-25 12:04         ` Petr Mladek
  0 siblings, 0 replies; 14+ messages in thread
From: Petr Mladek @ 2018-06-25 12:04 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Steven Rostedt, Thierry Escande, David Miller, Rasmus Villemoes,
	Tobin C . Harding, linux-kernel, Andy Shevchenko, Linus Torvalds

On Mon 2018-06-25 09:50:20, Petr Mladek wrote:
> On Fri 2018-06-22 23:50:20, Thierry Escande wrote:
> > On 22/06/2018 22:53, Steven Rostedt wrote:
> > > On Thu, Jun 07, 2018 at 02:24:34PM +0200, 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
> > > 
> > > And neither does my test box. It killed my tests I was running, as one of the
> > > configs I test has this set.
> > > 
> > > It appears that Andrew pulled it in and sent it to Linus, as it is in
> > > 4.18-rc1, and I need to now revert this patch to make my tests work.
> > 
> > This patch has been superseded with a v2 and a v3 pushed into Petr
> > printk.git tree (https://git.kernel.org/pub/scm/linux/kernel/git/pmladek/printk.git/commit/?h=for-4.19&id=ce041c43f22298485122bab15c14d062383fbc67).
> > Sorry for the mess...
> 
> Andrew,
> 
> should I send the revert and the better fix to Linus or would you like
> to do so?

Below is the proposed revert-commit just in case people want to add
Reviewed-by tags or so.


From 043f891b70e6197bc181f3b087c2bd04c60fddd2 Mon Sep 17 00:00:00 2001
From: Petr Mladek <pmladek@suse.com>
Date: Mon, 25 Jun 2018 13:28:06 +0200
Subject: [PATCH] Revert "lib/test_printf.c: call wait_for_random_bytes()
 before plain %p tests"

This reverts commit ee410f15b1418f2f4428e79980674c979081bcb7.

It might prevent the machine from boot. It would wait for enough
randomness at the very beginning of kernel_init(). But there is
basically nothing running in parallel that would help to produce
any randomness.

Reported-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 lib/test_printf.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index b2aa8f514844..cea592f402ed 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -260,13 +260,6 @@ plain(void)
 {
 	int err;
 
-	/*
-	 * Make sure crng is ready. Otherwise we get "(ptrval)" instead
-	 * of a hashed address when printing '%p' in plain_hash() and
-	 * plain_format().
-	 */
-	wait_for_random_bytes();
-
 	err = plain_hash();
 	if (err) {
 		pr_warn("plain 'p' does not appear to be hashed\n");
-- 
2.13.7


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-06-25 12:10 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-04 11:37 [PATCH RESEND] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests Thierry Escande
2018-06-05 12:43 ` Andy Shevchenko
2018-06-07 12:24 ` Petr Mladek
2018-06-07 18:47   ` Thierry Escande
2018-06-08  8:04     ` Petr Mladek
2018-06-08  9:07       ` Thierry Escande
2018-06-08  9:46         ` Andy Shevchenko
2018-06-08 10:32           ` Thierry Escande
2018-06-08 11:22             ` Petr Mladek
2018-06-08 11:28               ` Thierry Escande
2018-06-22 20:53   ` Steven Rostedt
2018-06-22 21:50     ` Thierry Escande
2018-06-25  7:50       ` Petr Mladek
2018-06-25 12:04         ` Petr Mladek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.