linux-kernel - [PATCH] random: 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-next>] [day] [month] [year] [list]
Date:   Fri, 29 Jul 2022 10:52:58 +0200
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     linux-kernel@...r.kernel.org
Cc:     "Jason A. Donenfeld" <Jason@...c4.com>,
        Theodore Ts'o <tytso@....edu>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        John Ogness <john.ogness@...utronix.de>,
        Mike Galbraith <efault@....de>, Petr Mladek <pmladek@...e.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: [PATCH] random: Initialize vsprintf's pointer hash once the random
 core is ready.

The printk code invokes vnsprintf in order to compute the complete
string before adding it into its buffer. This happens in an IRQ-off
region which leads to a warning on PREEMPT_RT in the random code if the
format strings contains a %p for pointer printing. This happens because
the random core acquires locks which become sleeping locks on PREEMPT_RT
which must not be acquired with disabled interrupts and or preemption.
By default the pointers are hashed which requires a random value on the
first invocation (either by printk or another user which comes first.

One could argue that there is no need for printk to disable interrupts
during the vsprintf() invocation which would fix the just mentioned
problem. However printk itself can be invoked in a context with
disabled interrupts which would lead to the very same problem.

This late init via printk can be avoided by explicitly initializing
vsprintf's random value once the random-core has been initialized.

Remove the on demand init from __ptr_to_hashval() and keep the -EAGAIN if
the init has not yet been performed. Move the actual init bits to
vsprintf_init_hash_pointer() which are invoked from random-core once it
has been initialized and get_random_bytes() is available.

Reported-by: Mike Galbraith <efault@....de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
---
 drivers/char/random.c  |  8 +++++++-
 include/linux/random.h |  2 ++
 lib/vsprintf.c         | 36 +++++++++++++++---------------------
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index a1af90bacc9f8..98f99026d1fba 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -202,6 +202,7 @@ static void extract_entropy(void *buf, size_t len);
 /* This extracts a new crng key from the input pool. */
 static void crng_reseed(void)
 {
+	bool init_hash_pointer = false;
 	unsigned long flags;
 	unsigned long next_gen;
 	u8 key[CHACHA_KEY_SIZE];
@@ -221,10 +222,15 @@ static void crng_reseed(void)
 		++next_gen;
 	WRITE_ONCE(base_crng.generation, next_gen);
 	WRITE_ONCE(base_crng.birth, jiffies);
-	if (!static_branch_likely(&crng_is_ready))
+	if (!static_branch_likely(&crng_is_ready)) {
 		crng_init = CRNG_READY;
+		init_hash_pointer = true;
+	}
 	spin_unlock_irqrestore(&base_crng.lock, flags);
 	memzero_explicit(key, sizeof(key));
+
+	if (init_hash_pointer)
+		vsprintf_init_hash_pointer();
 }
 
 /*
diff --git a/include/linux/random.h b/include/linux/random.h
index 20e389a14e5c7..229743ba5b4de 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -144,4 +144,6 @@ int random_online_cpu(unsigned int cpu);
 extern const struct file_operations random_fops, urandom_fops;
 #endif
 
+void vsprintf_init_hash_pointer(void);
+
 #endif /* _LINUX_RANDOM_H */
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3c1853a9d1c09..6fa2ebb9f9b9e 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -751,36 +751,30 @@ static int __init debug_boot_weak_hash_enable(char *str)
 early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable);
 
 static DEFINE_STATIC_KEY_FALSE(filled_random_ptr_key);
+static siphash_key_t ptr_key __read_mostly;
 
-static void enable_ptr_key_workfn(struct work_struct *work)
+void vsprintf_init_hash_pointer(void)
 {
-	static_branch_enable(&filled_random_ptr_key);
+	static DEFINE_SPINLOCK(filling);
+	unsigned long flags;
+	static bool filled;
+
+	spin_lock_irqsave(&filling, flags);
+	if (!filled) {
+		get_random_bytes(&ptr_key, sizeof(ptr_key));
+		filled = true;
+		static_branch_enable(&filled_random_ptr_key);
+	}
+	spin_unlock_irqrestore(&filling, flags);
 }
 
 /* Maps a pointer to a 32 bit unique identifier. */
 static inline int __ptr_to_hashval(const void *ptr, unsigned long *hashval_out)
 {
-	static siphash_key_t ptr_key __read_mostly;
 	unsigned long hashval;
 
-	if (!static_branch_likely(&filled_random_ptr_key)) {
-		static bool filled = false;
-		static DEFINE_SPINLOCK(filling);
-		static DECLARE_WORK(enable_ptr_key_work, enable_ptr_key_workfn);
-		unsigned long flags;
-
-		if (!system_unbound_wq || !rng_is_initialized() ||
-		    !spin_trylock_irqsave(&filling, flags))
-			return -EAGAIN;
-
-		if (!filled) {
-			get_random_bytes(&ptr_key, sizeof(ptr_key));
-			queue_work(system_unbound_wq, &enable_ptr_key_work);
-			filled = true;
-		}
-		spin_unlock_irqrestore(&filling, flags);
-	}
-
+	if (!static_branch_likely(&filled_random_ptr_key))
+		return -EAGAIN;
 
 #ifdef CONFIG_64BIT
 	hashval = (unsigned long)siphash_1u64((u64)ptr, &ptr_key);
-- 
2.36.1