From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>,
Thomas Gleixner <tglx@linutronix.de>,
Filipe Manana <fdmanana@suse.com>,
Peter Zijlstra <peterz@infradead.org>,
Borislav Petkov <bp@alien8.de>, Theodore Ts'o <tytso@mit.edu>
Subject: [PATCH 2/3] random: do not use input pool from hard IRQs
Date: Fri, 6 May 2022 23:54:53 +0200 [thread overview]
Message-ID: <20220506215454.1671-2-Jason@zx2c4.com> (raw)
In-Reply-To: <20220506215454.1671-1-Jason@zx2c4.com>
Years ago, a separate fast pool was added for interrupts, so that the
cost associated with taking the input pool spinlocks and mixing into it
would be avoided in places where latency is critical. However, one
oversight was that add_input_randomness() and add_disk_randomness()
still sometimes are called directly from the interrupt handler, rather
than being deferred to a thread. This means that some unlucky interrupts
will be caught doing a blake2s_compress() call and potentially spinning
on input_pool.lock, which can also be taken by unprivileged users by
writing into /dev/urandom.
We also observe that the entropy estimation algorithm uses jiffies,
which is rather coarse. In practice what this means that
add_timer_randomness() will credit the first event called from an
interrupt, but will skip the subsequent ones. However, if this is
triggered by an interrupt, the first event will *also* hit
add_interrupt_randomness(), which does its own crediting. So in essence
we double-count these inputs. So this commit not only avoids calling the
expensive functions from hard IRQ but also avoids this double counting.
In order to fix these issues, add_timer_randomness() now checks whether
it is being called from a hard IRQ and if so, just mixes into the
per-cpu IRQ fast pool using fast_mix(), which is much faster and can be
done lock-free. A nice consequence of this, as well, is that it means
hard IRQ context FPU support is likely no longer useful.
A downside of this, however, is that the num argument is potentially
attacker controlled, which puts a bit more pressure on the fast_mix()
sponge to do more than it's really intended to do. As a mitigating
factor, the first 96 bits of input aren't attacker controlled (a cycle
counter followed by zeros), which means it's essentially two rounds of
siphash rather than one, which is somewhat better. It's also not that
much different from add_interrupt_randomness()'s use of the irq stack
instruction pointer register.
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Filipe Manana <fdmanana@suse.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
drivers/char/random.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/char/random.c b/drivers/char/random.c
index d15f2133e804..818432638c18 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1227,6 +1227,12 @@ static void add_timer_randomness(struct timer_rand_state *state, unsigned int nu
unsigned long entropy = random_get_entropy(), now = jiffies, flags;
long delta, delta2, delta3;
+ if (in_hardirq()) {
+ fast_mix(this_cpu_ptr(&irq_randomness)->pool,
+ (unsigned long[2]){ entropy, num });
+ return;
+ }
+
spin_lock_irqsave(&input_pool.lock, flags);
_mix_pool_bytes(&entropy, sizeof(entropy));
_mix_pool_bytes(&num, sizeof(num));
--
2.35.1
next prev parent reply other threads:[~2022-05-06 21:55 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-06 21:54 [PATCH 1/3] random: order timer entropy functions below interrupt functions Jason A. Donenfeld
2022-05-06 21:54 ` Jason A. Donenfeld [this message]
2022-05-10 14:00 ` [PATCH v2] random: do not use input pool from hard IRQs Jason A. Donenfeld
2022-05-06 21:54 ` [PATCH 3/3] random: help compiler out with fast_mix() by using simpler arguments Jason A. Donenfeld
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220506215454.1671-2-Jason@zx2c4.com \
--to=jason@zx2c4.com \
--cc=bp@alien8.de \
--cc=fdmanana@suse.com \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®