mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] fault-inject: fix boot failure with fault injection boot parameters
@ 2026-11-05 16:01 Sang-Heon Jeon
  2026-11-10  5:11 ` Akinobu Mita
  2026-11-10  5:33 ` Andrew Morton
  0 siblings, 2 replies; 5+ messages in thread
From: Sang-Heon Jeon @ 2026-11-05 16:01 UTC (permalink / raw)
  To: Akinobu Mita, Andrew Morton; +Cc: linux-kernel, stable

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.

Fixes: e9bc360b105c ("fault-inject: use prandom where cryptographically secure randomness is not needed")
Cc: stable@vger.kernel.org
Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
---
QEMU-based test results
- build with gcc 16.2 + x86_64, booted with "failslab=1,0,-1,0"

1) AS-IS (before fix) 
[    0.090415] BUG: kernel NULL pointer dereference, address: 0000000000000018
[    0.091661] #PF: supervisor read access in kernel mode
[    0.092639] #PF: error_code(0x0000) - not-present page
[    0.093593] PGD 0 P4D 0 
[    0.094051] Oops: Oops: 0000 [#1] SMP NOPTI
[    0.094800] CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 7.3.0 #2 PREEMPT(undef) 
[    0.096178] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-debian-1.17.0-1ubuntu1 04/01/2014
[    0.098046] RIP: 0010:__kmalloc_cache_noprof+0x142/0x370
[    0.099024] Code: ea c1 ea 08 41 89 d5 41 83 e5 01 8b 53 1c 0f 1f 44 00 00 45 84 ed 74 12 4d 85 d2 74 0d 4c 89 d7 31 f6 e8 e1 2d e1 00 49 89 c2 <44> 8b 43 18 48 8b 74 24 50 0f 1fb
[    0.102388] RSP: 0000:ffffffff82c03d50 EFLAGS: 00010046
[    0.103326] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
[    0.104604] RDX: 0000000000000001 RSI: 0000000000000020 RDI: 0000000000000000
[    0.105883] RBP: 0000000000000820 R08: ffff88807dda6c70 R09: 46ad46184e3f2c64
[    0.107194] R10: 0000000000000000 R11: 46ad46184e3f2c64 R12: 0000000000000030
[    0.108472] R13: 0000000000000001 R14: 0000000000000008 R15: ffff88807ffd3ab1
[    0.109832] FS:  0000000000000000(0000) GS:ffff8880fa4ac000(0000) knlGS:0000000000000000
[    0.111284] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    0.112311] CR2: 0000000000000018 CR3: 0000000002c30000 CR4: 00000000000000b0
[    0.113616] Call Trace:
[    0.114059]  <TASK>
[    0.114462]  ? once_disable_jump+0x20/0x80
[    0.115202]  once_disable_jump+0x20/0x80
[    0.115905]  setup_fault_attr+0xe6/0xf0
[    0.116597]  unknown_bootoption+0x111/0x210
[    0.117352]  parse_args+0x14f/0x3c0
[    0.117983]  ? __pfx_unknown_bootoption+0x10/0x10
[    0.118839]  start_kernel+0x3b8/0x7c0
[    0.119539]  ? __pfx_unknown_bootoption+0x10/0x10
[    0.120435]  x86_64_start_reservations+0x24/0x30
[    0.121310]  x86_64_start_kernel+0xd1/0xe0
[    0.122278]  common_startup_64+0x13e/0x158
[    0.124133]  </TASK>
[    0.124756] Modules linked in:
[    0.125309] CR2: 0000000000000018
[    0.125905] ---[ end trace 0000000000000000 ]---
[    0.126741] RIP: 0010:__kmalloc_cache_noprof+0x142/0x370
[    0.127704] Code: ea c1 ea 08 41 89 d5 41 83 e5 01 8b 53 1c 0f 1f 44 00 00 45 84 ed 74 12 4d 85 d2 74 0d 4c 89 d7 31 f6 e8 e1 2d e1 00 49 89 c2 <44> 8b 43 18 48 8b 74 24 50 0f 1fb
[    0.131129] RSP: 0000:ffffffff82c03d50 EFLAGS: 00010046
[    0.132126] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
[    0.133483] RDX: 0000000000000001 RSI: 0000000000000020 RDI: 0000000000000000
[    0.134843] RBP: 0000000000000820 R08: ffff88807dda6c70 R09: 46ad46184e3f2c64
[    0.136206] R10: 0000000000000000 R11: 46ad46184e3f2c64 R12: 0000000000000030
[    0.137565] R13: 0000000000000001 R14: 0000000000000008 R15: ffff88807ffd3ab1
[    0.138928] FS:  0000000000000000(0000) GS:ffff8880fa4ac000(0000) knlGS:0000000000000000
[    0.142705] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    0.143741] CR2: 0000000000000018 CR3: 0000000002c30000 CR4: 00000000000000b0
[    0.145037] Kernel panic - not syncing: Attempted to kill the idle task!
[    0.146284] ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---2) TO-BE (after fix)

2) TO-BE (after fix)
- boot succeeded

---
 lib/fault-inject.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/fault-inject.c b/lib/fault-inject.c
index 02916ef2761c..9182d457baf5 100644
--- a/lib/fault-inject.c
+++ b/lib/fault-inject.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include <linux/once_lite.h>
 #include <linux/prandom.h>
 #include <linux/debugfs.h>
 #include <linux/sched.h>
@@ -50,7 +51,12 @@ int setup_fault_attr(struct fault_attr *attr, char *str)
 		return 0;
 	}
 
-	prandom_init_once(&fault_rnd_state);
+	/*
+	 * __setup handlers run before slab and workqueues are initialized,
+	 * but prandom_init_once() calls kmalloc() and schedule_work(). So use
+	 * DO_ONCE_LITE() instead.
+	 */
+	DO_ONCE_LITE(prandom_seed_full_state, &fault_rnd_state);
 
 	attr->probability = probability;
 	attr->interval = interval;

base-commit: c2366a2edc886a5a02d2a43de8b1c6ec09bab840
-- 
2.53.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fault-inject: fix boot failure with fault injection boot parameters
  2026-11-05 16:01 [PATCH] fault-inject: fix boot failure with fault injection boot parameters Sang-Heon Jeon
@ 2026-11-10  5:11 ` Akinobu Mita
  2026-11-10  5:33 ` Andrew Morton
  1 sibling, 0 replies; 5+ messages in thread
From: Akinobu Mita @ 2026-11-10  5:11 UTC (permalink / raw)
  To: Sang-Heon Jeon; +Cc: Andrew Morton, linux-kernel, stable

2026年11月6日(金) 1:02 Sang-Heon Jeon <ekffu200098@gmail.com>:
>
> 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.
>
> Fixes: e9bc360b105c ("fault-inject: use prandom where cryptographically secure randomness is not needed")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>

Thank you very much for fixing the problem.

Reviewed-by: Akinobu Mita <akinobu.mita@gmail.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fault-inject: fix boot failure with fault injection boot parameters
  2026-11-05 16:01 [PATCH] fault-inject: fix boot failure with fault injection boot parameters Sang-Heon Jeon
  2026-11-10  5:11 ` Akinobu Mita
@ 2026-11-10  5:33 ` Andrew Morton
  2026-11-10 11:36   ` Sang-Heon Jeon
  1 sibling, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2026-11-10  5:33 UTC (permalink / raw)
  To: Sang-Heon Jeon; +Cc: Akinobu Mita, linux-kernel, stable

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);
 }

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fault-inject: fix boot failure with fault injection boot parameters
  2026-11-10  5:33 ` Andrew Morton
@ 2026-11-10 11:36   ` Sang-Heon Jeon
  2026-11-10 20:43     ` Andrew Morton
  0 siblings, 1 reply; 5+ messages in thread
From: Sang-Heon Jeon @ 2026-11-10 11:36 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Akinobu Mita, linux-kernel, stable

Hi, Andrew

2026년 11월 10일 (화) 오후 2:33, Andrew Morton <akpm@linux-foundation.org>님이 작성:
>
> 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 :(

Thanks for looking into this. I checked the proposal from LLM.

>
>
>
> 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.

In theory, right. But IMHO, do we really need serialization in
setup_fault_attr()?

> 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.

Right.

> 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.

Right, and that's what we are trying to fix.

> 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.

Reseeding could be useless work, but it does not break the system.

> 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.

Code description is correct. But we can easily find how the code works
without this description..

> 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.

Again, right, but no need to describe this in the commit message.

> 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;

declare below fault_rnd_state looks better.

> +
> +       /*
> +        * 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);
>  }

In general, the shared initialization helper looks okay. But I think
this should be applied as a separate patch. Also, the commit message
is a bit verbose.

Andrew, is it okay to send an extra patch based on your suggestion
with your co-developed-by tag?

Best regards,
Sang-Heon Jeon

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fault-inject: fix boot failure with fault injection boot parameters
  2026-11-10 11:36   ` Sang-Heon Jeon
@ 2026-11-10 20:43     ` Andrew Morton
  0 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2026-11-10 20:43 UTC (permalink / raw)
  To: Sang-Heon Jeon; +Cc: Akinobu Mita, linux-kernel, stable

On Tue, 10 Nov 2026 20:36:54 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:

> 
> Code description is correct. But we can easily find how the code works
> without this description..

Yup ;)

>
> Andrew, is it okay to send an extra patch based on your suggestion
> with your co-developed-by tag?

Please do whatever you prefer.  There's no need to attribute AI stuff to
me - I was just sharing in case someone found it useful.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-10 20:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-05 16:01 [PATCH] fault-inject: fix boot failure with fault injection boot parameters Sang-Heon Jeon
2026-11-10  5:11 ` Akinobu Mita
2026-11-10  5:33 ` Andrew Morton
2026-11-10 11:36   ` Sang-Heon Jeon
2026-11-10 20:43     ` Andrew Morton

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®