From: Andrew Morton <akpm@linux-foundation.org>
To: Sang-Heon Jeon <ekffu200098@gmail.com>
Cc: Akinobu Mita <akinobu.mita@gmail.com>,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] fault-inject: fix boot failure with fault injection boot parameters
Date: Mon, 9 Nov 2026 21:33:46 -0800 [thread overview]
Message-ID: <20261109213346.634e83b158ee7086e64f9cd5@linux-foundation.org> (raw)
In-Reply-To: <20261105160209.1504867-1-ekffu200098@gmail.com>
On Fri, 6 Nov 2026 01:01:58 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> Since commit e9bc360b105c ("fault-inject: use prandom where
> cryptographically secure randomness is not needed"), setup_fault_attr()
> calls prandom_init_once(), which calls kmalloc() and schedule_work().
>
> But setup_fault_attr() can be called from __setup handlers before slab
> and workqueues are initialized, so any fault injection boot parameter
> leads to boot failure.
>
> So use DO_ONCE_LITE() instead.
thanks. I'm surprised that command-line fault-injection options (or at
least failslab) have been broken for 1.5 years.
And what's going on in that code? Why does it need to call
prandom_init_once() from three different sites?
I discussed it with LLM for a while and it proposed the below. But I
ran out of time to think about this. The code seems to need a
signficant rethink so I suppose a quick bandaid as you suggest is good
enough for now :(
Could we use a shared initialization helper instead?
There are three prandom_init_once() expansions in fault-inject.c.
Each has its own guard, so configuring another interface can reseed
the same state after fault injection has started.
DO_ONCE_LITE() avoids the allocation and workqueue dependency, but
doesn't serialize concurrent initialization or wait for another caller
to finish seeding.
The patch below keeps prandom_seed_full_state() and gives all three
configuration paths one lock and one completion flag. The three calls
remain because each interface can independently enable injection.
Moving initialization to an initcall would leave boot-parameter users
able to inject faults before the initcall runs.
An ordinary spinlock is sufficient: these are configuration paths, not
the interrupt-time random-number generation path. It also preserves
the lock type used by DO_ONCE(), allowing the RNG's ordinary locks on
PREEMPT_RT. No raw spinlock or explicit interrupt disabling is needed.
From: Andrew Morton <akpm@linux-foundation.org>
Date: Thu, 5 Nov 2026 10:01:33 -0800
Subject: [PATCH] fault-inject: initialize random state once without workqueues
Fault-injection boot parameters call setup_fault_attr() before slab
and workqueues are initialized. Its prandom_init_once() call expands
to DO_ONCE(), whose static-key disabling path allocates memory and
schedules work. A valid boot parameter can therefore crash the kernel
before boot completes, even when its probability is zero.
The debugfs and configfs setup paths have separate prandom_init_once()
expansions. Their independent guards also allow the shared random
state to be seeded again after another interface has enabled fault
injection.
Replace the three expansions with calls to a shared helper. Serialize
initialization with an ordinary spinlock and set the completion flag
only after prandom_seed_full_state() has finished. This retains the
existing seeding algorithm without allocating memory or scheduling
work to disable a static key.
The lock protects configuration-time initialization, which is not
called from interrupt handlers. Use an ordinary spinlock rather than
a raw spinlock because the RNG acquires ordinary locks that can sleep
on PREEMPT_RT.
Fixes: e9bc360b105c ("fault-inject: use prandom where cryptographically secure randomness is not needed")
Reported-by: Sang-Heon Jeon <ekffu200098@gmail.com>
Cc: Akinobu Mita <akinobu.mita@gmail.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
Prepared against c2366a2edc886a5a02d2a43de8b1c6ec09bab840.
Source inspection only; this replacement has not been built or
boot-tested.
lib/fault-inject.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/lib/fault-inject.c b/lib/fault-inject.c
--- a/lib/fault-inject.c
+++ b/lib/fault-inject.c
@@ -12,6 +12,7 @@
#include <linux/interrupt.h>
#include <linux/stacktrace.h>
#include <linux/fault-inject.h>
+#include <linux/spinlock.h>
/*
* The should_fail() functions use prandom instead of the normal Linux RNG
@@ -19,6 +20,24 @@
*/
static DEFINE_PER_CPU(struct rnd_state, fault_rnd_state);
+static void fault_random_init(void)
+{
+ static DEFINE_SPINLOCK(lock);
+ static bool initialized;
+
+ /*
+ * Configuration paths must finish seeding before enabling injection,
+ * and must not reseed state already in use by another interface.
+ * These callers are not interrupt handlers. An ordinary lock allows
+ * the RNG's sleeping locks on PREEMPT_RT.
+ */
+ spin_lock(&lock);
+ if (!initialized) {
+ prandom_seed_full_state(&fault_rnd_state);
+ initialized = true;
+ }
+ spin_unlock(&lock);
+}
+
static u32 fault_prandom_u32_below_100(void)
{
struct rnd_state *state;
@@ -50,7 +69,7 @@
return 0;
}
- prandom_init_once(&fault_rnd_state);
+ fault_random_init();
attr->probability = probability;
attr->interval = interval;
@@ -240,7 +259,7 @@
if (IS_ERR(dir))
return dir;
- prandom_init_once(&fault_rnd_state);
+ fault_random_init();
debugfs_create_ul("probability", mode, dir, &attr->probability);
debugfs_create_ul("interval", mode, dir, &attr->interval);
@@ -456,7 +475,7 @@
void fault_config_init(struct fault_config *config, const char *name)
{
- prandom_init_once(&fault_rnd_state);
+ fault_random_init();
config_group_init_type_name(&config->group, name, &fault_config_type);
}
prev parent reply other threads:[~2026-11-10 5:33 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-05 16:01 Sang-Heon Jeon
2026-11-10 5:11 ` Akinobu Mita
2026-11-10 5:33 ` Andrew Morton [this message]
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=20261109213346.634e83b158ee7086e64f9cd5@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=akinobu.mita@gmail.com \
--cc=ekffu200098@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
/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®