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:   Tue, 16 Aug 2022 16:02:55 +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

Hey Sebastian,

On Thu, Aug 11, 2022 at 04:20:21PM +0200, Jason A. Donenfeld wrote:
> 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.

Thought I'd ping you about this again...

Jason