1. Patchew
  2. linux
  3. View series

[PATCH 0/2 v4] Init the hashed pointer from a worker.

Sebastian Andrzej Siewior posted 2 patches 3 years, 6 months ago 
|
[PATCH 0/2 v4] Init the hashed pointer from a worker.
Posted by Sebastian Andrzej Siewior 3 years, 6 months ago 
This is a mini series to initialize the random value, needed for the %p
format argument, upfront during boot instead on demand. The latter is
problematic on PREEMPT_RT if the first user happens to be in an atomic
region.

v3…v4:
    - Added a __read_mostly.
    - Added Jason's Acked-by for 2/2 after talking to him at Plumbers.
      While we were discussion several ways of tackling this differently
      and the possible problems/ side effects that this may cause we
      happen to notice that the current way of doing things is also a
      problem if the first printk("%p\n") user happens to be in NMI
      context.
      Therefore I leave it to the vsprintf/ printk maintainer to decide
      if this is -stable material or not. I'm not aware of any NMI code
      path using %p but then it is not officially forbidden.
      Assuming unknown_nmi_error() contains %p format the string, then
      the backtrace at the end of the email will be printed.

v2…v3:
    - schedule a worker every two seconds if the RNG core is not ready.
    https://lore.kernel.org/all/YueeIgPGUJgsnsAh@linutronix.de

v1…v2:
   - Remove the static_branch_likely() as suggested by Petr Mladek.
   - Jason wasn't onboard with fiddling in random core to get the job
     done. Instead a worker is scheduled from an initcall and
     get_random_bytes_wait() is used to get the date once it is
     available.
   https://lore.kernel.org/all/20220729154716.429964-1-bigeasy@linutronix.de/

v1:
   https://lore.kernel.org/all/YuOf6qu453dOkR+S@linutronix.de/

Before the series after adding "%p" to unknown_nmi_error() and
triggering a NMI:
| ================================
| WARNING: inconsistent lock state
| 6.0.0-rc7+ #6 Not tainted
| --------------------------------
| inconsistent {INITIAL USE} -> {IN-NMI} usage.
| swapper/0/0 [HC1[1]:SC0[0]:HE0:SE1] takes:
| ffffffff82aea4f0 (input_pool.lock){..-.}-{2:2}, at: extract_entropy.constprop.0+0x76/0x240
| {INITIAL USE} state was registered at:
| irq event stamp: 37104
| hardirqs last  enabled at (37103): [<ffffffff81cc8e80>] default_idle_call+0x20/0x90
| hardirqs last disabled at (37104): [<ffffffff81cbbb2b>] exc_nmi+0x7b/0x120
| softirqs last  enabled at (37098): [<ffffffff810f3a8c>] __irq_exit_rcu+0x8c/0xb0
| softirqs last disabled at (37085): [<ffffffff810f3a8c>] __irq_exit_rcu+0x8c/0xb0
| 
| other info that might help us debug this:
|  Possible unsafe locking scenario:
| 
|        CPU0
|        ----
|   lock(input_pool.lock);
|   <Interrupt>
|     lock(input_pool.lock);
| 
|  *** DEADLOCK ***
| 
| no locks held by swapper/0/0.
| 
| stack backtrace:
| CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.0.0-rc7+ #6
| Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-debian-1.16.0-4 04/01/2014
| Call Trace:
|  <NMI>
|  dump_stack_lvl+0x4c/0x63
|  lock_acquire.cold+0x43/0x48
|  _raw_spin_lock_irqsave+0x33/0x50
|  extract_entropy.constprop.0+0x76/0x240
|  crng_reseed+0x20/0xf0
|  crng_make_state+0x51/0x2b0
|  _get_random_bytes.part.0+0x47/0x150
|  default_pointer+0x3e9/0x420
|  vsnprintf+0x1a8/0x550
|  vprintk_store+0x13e/0x4c0
|  vprintk+0x2e/0x50
|  _printk+0x53/0x6e
|  default_do_nmi+0x224/0x290
|  exc_nmi+0xf1/0x120
|  end_repeat_nmi+0x16/0x67
| RIP: 0010:default_idle+0xb/0x10
…
|  </NMI>
|  <TASK>
|  default_idle_call+0x51/0x90
|  do_idle+0x201/0x270
|  cpu_startup_entry+0x14/0x20
|  rest_init+0xe5/0x170
|  arch_call_rest_init+0x5/0xa
|  start_kernel+0x68c/0x6b5
|  secondary_startup_64_no_verify+0xe0/0xeb
|  </TASK>
| ------------[ cut here ]------------
| WARNING: CPU: 0 PID: 0 at arch/x86/kernel/fpu/core.c:60 irq_fpu_usable+0x34/0x40
| Modules linked in:
| CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.0.0-rc7+ #6
| Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-debian-1.16.0-4 04/01/2014
| RIP: 0010:irq_fpu_usable+0x34/0x40
…
| Call Trace:
|  <NMI>
|  blake2s_compress+0x1b/0xa0
|  blake2s_final+0x3c/0x60
|  extract_entropy.constprop.0+0x8d/0x240
|  crng_reseed+0x20/0xf0
|  crng_make_state+0x51/0x2b0
|  _get_random_bytes.part.0+0x47/0x150
|  default_pointer+0x3e9/0x420
|  vsnprintf+0x1a8/0x550
|  vprintk_store+0x13e/0x4c0
|  vprintk+0x2e/0x50
|  _printk+0x53/0x6e
|  default_do_nmi+0x224/0x290
|  exc_nmi+0xf1/0x120
|  end_repeat_nmi+0x16/0x67
| RIP: 0010:default_idle+0xb/0x10
…
|  </NMI>
|  <TASK>
|  default_idle_call+0x51/0x90
|  do_idle+0x201/0x270
|  cpu_startup_entry+0x14/0x20
|  rest_init+0xe5/0x170
|  arch_call_rest_init+0x5/0xa
|  start_kernel+0x68c/0x6b5
|  secondary_startup_64_no_verify+0xe0/0xeb
|  </TASK>
| irq event stamp: 37104
| hardirqs last  enabled at (37103): [<ffffffff81cc8e80>] default_idle_call+0x20/0x90
| hardirqs last disabled at (37104): [<ffffffff81cbbb2b>] exc_nmi+0x7b/0x120
| softirqs last  enabled at (37098): [<ffffffff810f3a8c>] __irq_exit_rcu+0x8c/0xb0
| softirqs last disabled at (37085): [<ffffffff810f3a8c>] __irq_exit_rcu+0x8c/0xb0
| ---[ end trace 0000000000000000 ]---
| Uhhuh. NMI received for unknown reason 30 on CPU 0 / 00000000f8da9c8a.
| Dazed and confused, but trying to continue

Sebastian
Re: [PATCH 0/2 v4] Init the hashed pointer from a worker.
Posted by Petr Mladek 3 years, 6 months ago 
On Tue 2022-09-27 12:49:10, Sebastian Andrzej Siewior wrote:
> This is a mini series to initialize the random value, needed for the %p
> format argument, upfront during boot instead on demand. The latter is
> problematic on PREEMPT_RT if the first user happens to be in an atomic
> region.
> 
> v3…v4:
>     - Added a __read_mostly.
>     - Added Jason's Acked-by for 2/2 after talking to him at Plumbers.
>       While we were discussion several ways of tackling this differently
>       and the possible problems/ side effects that this may cause we
>       happen to notice that the current way of doing things is also a
>       problem if the first printk("%p\n") user happens to be in NMI
>       context.
>       Therefore I leave it to the vsprintf/ printk maintainer to decide
>       if this is -stable material or not. I'm not aware of any NMI code
>       path using %p but then it is not officially forbidden.
>       Assuming unknown_nmi_error() contains %p format the string, then
>       the backtrace at the end of the email will be printed.

JFYI, both patches are committed into printk/linux.git,
branch for-6.1-hash-pointer-init

I have just added a note into the 2nd patch about that it also
prevents deadlock when printk("%p", ptr) is called under the lock
used by get_random_bytes(), see
https://git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git/commit/?h=for-6.1-hash-pointer-init&id=6f0ac3b52a9075b7291a72fb338d08491c1f0a64

I did not modify the code. The system_unbound_wq can and should be
fixed separately.

Best Regards,
Petr

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.