The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: linux-kernel@vger.kernel.org,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	John Ogness <john.ogness@linutronix.de>,
	Mike Galbraith <efault@gmx.de>, Petr Mladek <pmladek@suse.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Theodore Ts'o <tytso@mit.edu>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: [PATCH 2/2 v3] lib/vsprintf: Initialize vsprintf's pointer hash once the random core is ready.
Date: Mon, 1 Aug 2022 11:34:26 +0200	[thread overview]
Message-ID: <YueeIgPGUJgsnsAh@linutronix.de> (raw)
In-Reply-To: <YueBnnnnvpxxjE4N@linutronix.de>

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 initialization 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() to retrieve the random value if the RNG
core is ready, otherwise schedule a worker in two seconds and try again.

Reported-by: Mike Galbraith <efault@gmx.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
v2…v3:
   - schedule a worker every two seconds if the RNG core is not ready.

 lib/vsprintf.c |   46 +++++++++++++++++++++++++++-------------------
 1 file changed, 27 insertions(+), 19 deletions(-)

--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -751,31 +751,39 @@ 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);
+static DECLARE_DELAYED_WORK(fill_ptr_key_work, fill_ptr_key_workfn);
+
+static void fill_ptr_key_workfn(struct work_struct *work)
+{
+	if (!rng_is_initialized()) {
+		queue_delayed_work(system_unbound_wq, &fill_ptr_key_work, HZ  * 2);
+		return;
+	}
+
+	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);
+}
+
+static int __init vsprintf_init_hashval(void)
+{
+	fill_ptr_key_workfn(NULL);
+	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 writing ptr_key. */
 	smp_rmb();
 

  reply	other threads:[~2022-08-01  9:34 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-29 15:47 [PATCH 0/2 v2] Init the hashed pointer from a worker Sebastian Andrzej Siewior
2022-07-29 15:47 ` [PATCH 1/2 v2] lib/vsprintf: Remove static_branch_likely() from __ptr_to_hashval() Sebastian Andrzej Siewior
2022-08-01 12:11   ` Jason A. Donenfeld
2022-08-01 12:41     ` Sebastian Andrzej Siewior
2022-08-01 12:13   ` Jason A. Donenfeld
2022-08-01 12:42     ` Sebastian Andrzej Siewior
2022-07-29 15:47 ` [PATCH 2/2 v2] lib/vsprintf: Initialize vsprintf's pointer hash once the random core is ready Sebastian Andrzej Siewior
2022-07-29 23:29   ` Jason A. Donenfeld
2022-08-01  7:32     ` Sebastian Andrzej Siewior
2022-08-01  9:34       ` Sebastian Andrzej Siewior [this message]
2022-08-01 12:36         ` [PATCH 2/2 v3] " Jason A. Donenfeld
2022-08-01 12:39           ` [PATCH v4] lib/vsprintf: defer filling siphash key on RT Jason A. Donenfeld
2022-08-01 12:46             ` Sebastian Andrzej Siewior
2022-08-01 13:36               ` Jason A. Donenfeld
2022-08-01 13:44                 ` Jason A. Donenfeld
2022-08-01 14:25                   ` Sebastian Andrzej Siewior
2022-08-01 14:30                     ` Jason A. Donenfeld
2022-08-01 13:47                 ` Sebastian Andrzej Siewior
2022-08-01 13:55                   ` Jason A. Donenfeld
2022-08-01 14:12                     ` [PATCH v5] " Jason A. Donenfeld
2022-08-01 14:26                       ` Sebastian Andrzej Siewior
2022-08-01 12:41           ` [PATCH 2/2 v3] lib/vsprintf: Initialize vsprintf's pointer hash once the random core is ready Sebastian Andrzej Siewior
2022-09-20 15:01         ` Jason A. Donenfeld
2022-09-23 10:36           ` Petr Mladek
2022-09-23 15:28             ` Sebastian Andrzej Siewior

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=YueeIgPGUJgsnsAh@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=Jason@zx2c4.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=efault@gmx.de \
    --cc=john.ogness@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.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