linux-kernel - [PATCH 2/2 v2] lib/vsprintf: 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-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20220729154716.429964-3-bigeasy@linutronix.de>
Date:   Fri, 29 Jul 2022 17:47:16 +0200
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     linux-kernel@...r.kernel.org
Cc:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        "Jason A . Donenfeld" <Jason@...c4.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>,
        Theodore Ts'o <tytso@....edu>,
        Thomas Gleixner <tglx@...utronix.de>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Subject: [PATCH 2/2 v2] lib/vsprintf: 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
disabled.
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.

Move the initializaion of ptr_key into a worker and schedule it from
subsys_initcall(). This happens early but after the workqueue subsystem
is ready. Use get_random_bytes_wait() to retrieve the random value which
will block until random data is available.

Reported-by: Mike Galbraith <efault@....de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
---
 lib/vsprintf.c |   44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -751,31 +751,37 @@ static int __init debug_boot_weak_hash_e
 early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable);
 
 static bool filled_random_ptr_key;
+static siphash_key_t ptr_key __read_mostly;
+
+static void fill_ptr_key_workfn(struct work_struct *work)
+{
+	int ret;
+
+	ret = get_random_bytes_wait(&ptr_key, sizeof(ptr_key));
+	if (WARN_ON(ret < 0))
+		return;
+	/* Pairs with smp_rmb() before reading ptr_key. */
+	smp_wmb();
+	WRITE_ONCE(filled_random_ptr_key, true);
+}
+
+static int vsprintf_init_hashval(void)
+{
+	static DECLARE_WORK(fill_ptr_key_work, fill_ptr_key_workfn);
+
+	queue_work(system_unbound_wq, &fill_ptr_key_work);
+	return 0;
+}
+subsys_initcall(vsprintf_init_hashval)
 
 /* 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 (!READ_ONCE(filled_random_ptr_key)) {
-		static bool filled = false;
-		static DEFINE_SPINLOCK(filling);
-		unsigned long flags;
-
-		if (!rng_is_initialized() ||
-		    !spin_trylock_irqsave(&filling, flags))
-			return -EAGAIN;
-
-		if (!filled) {
-			get_random_bytes(&ptr_key, sizeof(ptr_key));
-			/* Pairs with smp_rmb() before reading ptr_key. */
-			smp_wmb();
-			WRITE_ONCE(filled_random_ptr_key, true);
-			filled = true;
-		}
-		spin_unlock_irqrestore(&filling, flags);
-	}
+	if (!READ_ONCE(filled_random_ptr_key))
+		return -EBUSY;
+
 	/* Pairs with smp_wmb() after writting ptr_key. */
 	smp_rmb();