linux-kernel - Re: [PATCH v4 2/2] lib/vsprintf: Initialize vsprintf's pointer hash once the random core 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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YzQP2OEVKgWtwsD4@linutronix.de>
Date:   Wed, 28 Sep 2022 11:11:52 +0200
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     Petr Mladek <pmladek@...e.com>
Cc:     linux-kernel@...r.kernel.org, Theodore Ts'o <tytso@....edu>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        "Jason A . Donenfeld " <Jason@...c4.com>,
        John Ogness <john.ogness@...utronix.de>,
        Mike Galbraith <efault@....de>,
        Peter Zijlstra <peterz@...radead.org>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v4 2/2] lib/vsprintf: Initialize vsprintf's pointer hash
 once the random core is ready.

On 2022-09-27 18:40:15 [+0200], Petr Mladek wrote:
> Another advantage is that it removes a nested lock from the printk()
> code path. A deadlock was partly prevented by the trylock. But there was
> still a risk of a deadlock when printk() was called under base_crng.lock.

Okay.

> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index bce63cbf23779..44b39ba56b796 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
…
> I have tried it and system_wq works well here. It actually
> even initializes the hash earlier here. But it is only by chance
> because it happens on the 2nd attempt instead of 3rd one.

Yeah. I added a reschedule of two seconds since it looked okay and I
didn't want to do very often. I have an old box where it takes ~12 secs
to setup and here it is the fifth attempt on average. (Before the rework
it needed way longer to initialize).

> > +		return;
> > +	}
> > +
> > +	get_random_bytes(&ptr_key, sizeof(ptr_key));
> > +
> > +	/* Pairs with smp_rmb() before reading ptr_key. */
> > +	smp_wmb();
> > +	WRITE_ONCE(filled_random_ptr_key, true);
> > +}
> 
> With "system_wq":
> 
> Reviewed-by: Petr Mladek <pmladek@...e.com>
> 
> I could replace "system_unbound_wq" with "system_wq" when
> pushing. Is anybody against it, please?

so schedule_delayed_work() then?
I don't mind at all. I used that one just because serialisation is not
needed and neither is the CPU important.

If you are going to replace it, then I am not going to send an update
(unless I'm old otherwise).

> I am sorry that I have missed it when looking at the previous
> version.

No worries.

> Best Regards,
> Petr

Sebastian