linux-kernel - Re: [PATCH] random: use raw spinlocks for use on RT

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]

Date:   Thu, 11 Aug 2022 16:20:21 +0200
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:     linux-kernel@...r.kernel.org, tglx@...utronix.de
Subject: Re: [PATCH] random: use raw spinlocks for use on RT

Hi Sebastian,

On Thu, Aug 11, 2022 at 09:15:11AM +0200, Sebastian Andrzej Siewior wrote:
> On 2022-08-11 02:17:31 [+0200], Jason A. Donenfeld wrote:
> > Hey Sebastian,
> Hi Jason,
> 
> > > > Sebastian - I won't move forward with this without your Ack, obviously.
> > > > What do you think of this general approach? -Jason
> > > 
> > > I would need to do worst-case measurements and I've been looking at this
> > > just before writting the other email and there was a local_lock_t
> > > somewhere which needs also change…
> > 
> > Did you ever come up some measurements here? It sure would be nice if I
> > could apply this, but obviously that's contingent on you saying it's
> > okay latency-wise on RT.
> 
> No, I did not. But I've been thinking a little about it. The worst case
> latency is important now and later.
> Looking at it, all we need is one init in vsprintf at boot time and we
> are done. That is the third fallout that I am aware of since the rework
> of get_random_*().
> We managed to get rid of all memory allocations (including GFP_ATOMIC)
> from preempt/IRQ-off section on PREEMPT_RT. Therefore I am not convinced
> to make all locks in random core a raw_spinlock_t just to make things
> work here as of now.

By grouping everything into "the rework of get_random_*()", you miss
important subtleties, as I mentioned before. Importantly, in this case,
the issue we're facing has absolutely nothing at all to do with that,
but is rather entirely the result of removing the async notifier
mechanism in favor of doing things more directly, more straight
forwardly. So let's not muddle what we're discussing here.

But more generally, the RNG is supposed to be usable from any context.
And adding wild workarounds, or worse, adding back complex async
notifier stuff, seems bad. So far your proposals for the printk issue
haven't been acceptable at all.

So why don't we actually fix this, so we don't have to keep coming up
with hacks? The question is: does using raw spinlocks over this code
result in any real issue for RT latency? If so, I'd like to know where,
and maybe I can do something about that (or maybe I can't). If not, then
this is a non problem and I'll apply this patch with your blessing.

If you don't want to spend time doing latency measurements, could you
instead share a document or similar to the type of methodology you
usually use for that, so I can do the same? And at the very least, I am
simply curious and want to know more about the RT world.

Jason