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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YufmHsYX0+I3rpx4@zx2c4.com>
Date:   Mon, 1 Aug 2022 16:41:34 +0200
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH] random: use raw spinlocks for use on RT

Hi Sebastian,

On Mon, Aug 01, 2022 at 04:34:13PM +0200, Sebastian Andrzej Siewior wrote:
> So I have everything ready for 5.20 (6.0) ready without the RT patch and
> then this vsprintf issues comes along…

I already sent my rc1 pull to Linus for 6.0, but I can do a second pull
with this in it for next week while the merge window is still open, so
that your RT-patch-len-zero objective is still met for 6.0.

> From that point of view I would prefer to either init it upfront in a
> way that works for everyone/ loose the first %p since it is probably a
> minor inconvenience if nobody complains - instead swapping all locks.
> We managed without this for kasan and lockdep which are both not used in
> a production environment.

The kfence change was a production change, actually. Lots of people turn
that on by default.

If you want to address this within printk itself, just do `if (rt || lockdep)`
as the condition, so we don't swallow the first one. When you have to
make code worse to satisfy a tool, the tool is the problem. We only
would need this first message dropping on rt, not on other kernels.
Don't knock other kernels.

However... I suspect these issues will continue to bite us in new subtle
ways for some time to come. Who is to say that you can't call
get_random_bytes() from a driver's hard IRQ? As RT gets integrated and
more widely deployed, I imagine these things will start coming up.
random.c was already designed to handle random bytes in irqoff; that's
why it uses irqsave/irqrestore all over its spinlock handling. This RT
thing is a snag in that original intention. But its an intention trivial
to recover with this patch. So if you're okay with it, I think I'd
prefer to do this and have our problems go away once and for all.

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

That would be very interesting to learn about. If your measurements say
yes, then maybe we can do this. If your measurements say "yikes", then I
guess we can't. Either way, I like having some metric to decide this by.

Jason