On 2022-07-13 15:48, Jason A. Donenfeld wrote:
Hi Harald,
On Wed, Jul 13, 2022 at 03:17:21PM +0200, Harald Freudenberger wrote:
This patch slightly reworks the s390 arch_get_random_seed_{int,long}
implementation: Make sure the CPACF trng instruction is never
called in any interrupt context. This is done by adding an
additional condition in_task().
Justification:
There are some constrains to satisfy for the invocation of the
arch_get_random_seed_{int,long}() functions:
- They should provide good random data during kernel initialization.
- They should not be called in interrupt context as the TRNG
instruction is relatively heavy weight and may for example
make some network loads cause to timeout and buck.
However, it was not clear what kind of interrupt context is exactly
encountered during kernel init or network traffic eventually calling
arch_get_random_seed_long().
After some days of investigations it is clear that the s390
start_kernel function is not running in any interrupt context and
so the trng is called:
Jul 11 18:33:39 t35lp54 kernel: [<00000001064e90ca>]
arch_get_random_seed_long.part.0+0x32/0x70
Jul 11 18:33:39 t35lp54 kernel: [<000000010715f246>]
random_init+0xf6/0x238
Jul 11 18:33:39 t35lp54 kernel: [<000000010712545c>]
start_kernel+0x4a4/0x628
Jul 11 18:33:39 t35lp54 kernel: [<000000010590402a>]
startup_continue+0x2a/0x40
The condition in_task() is true and the CPACF trng provides random
data
during kernel startup.
The network traffic however, is more difficult. A typical call stack
looks like this:
Jul 06 17:37:07 t35lp54 kernel: [<000000008b5600fc>]
extract_entropy.constprop.0+0x23c/0x240
Jul 06 17:37:07 t35lp54 kernel: [<000000008b560136>]
crng_reseed+0x36/0xd8
Jul 06 17:37:07 t35lp54 kernel: [<000000008b5604b8>]
crng_make_state+0x78/0x340
Jul 06 17:37:07 t35lp54 kernel: [<000000008b5607e0>]
_get_random_bytes+0x60/0xf8
Jul 06 17:37:07 t35lp54 kernel: [<000000008b56108a>]
get_random_u32+0xda/0x248
Jul 06 17:37:07 t35lp54 kernel: [<000000008aefe7a8>]
kfence_guarded_alloc+0x48/0x4b8
Jul 06 17:37:07 t35lp54 kernel: [<000000008aeff35e>]
__kfence_alloc+0x18e/0x1b8
Jul 06 17:37:07 t35lp54 kernel: [<000000008aef7f10>]
__kmalloc_node_track_caller+0x368/0x4d8
Jul 06 17:37:07 t35lp54 kernel: [<000000008b611eac>]
kmalloc_reserve+0x44/0xa0
Jul 06 17:37:07 t35lp54 kernel: [<000000008b611f98>]
__alloc_skb+0x90/0x178
Jul 06 17:37:07 t35lp54 kernel: [<000000008b6120dc>]
__napi_alloc_skb+0x5c/0x118
Jul 06 17:37:07 t35lp54 kernel: [<000000008b8f06b4>]
qeth_extract_skb+0x13c/0x680
Jul 06 17:37:07 t35lp54 kernel: [<000000008b8f6526>]
qeth_poll+0x256/0x3f8
Jul 06 17:37:07 t35lp54 kernel: [<000000008b63d76e>]
__napi_poll.constprop.0+0x46/0x2f8
Jul 06 17:37:07 t35lp54 kernel: [<000000008b63dbec>]
net_rx_action+0x1cc/0x408
Jul 06 17:37:07 t35lp54 kernel: [<000000008b937302>]
__do_softirq+0x132/0x6b0
Jul 06 17:37:07 t35lp54 kernel: [<000000008abf46ce>]
__irq_exit_rcu+0x13e/0x170
Jul 06 17:37:07 t35lp54 kernel: [<000000008abf531a>]
irq_exit_rcu+0x22/0x50
Jul 06 17:37:07 t35lp54 kernel: [<000000008b922506>]
do_io_irq+0xe6/0x198
Jul 06 17:37:07 t35lp54 kernel: [<000000008b935826>]
io_int_handler+0xd6/0x110
Jul 06 17:37:07 t35lp54 kernel: [<000000008b9358a6>]
psw_idle_exit+0x0/0xa
Jul 06 17:37:07 t35lp54 kernel: ([<000000008ab9c59a>]
arch_cpu_idle+0x52/0xe0)
Jul 06 17:37:07 t35lp54 kernel: [<000000008b933cfe>]
default_idle_call+0x6e/0xd0
Jul 06 17:37:07 t35lp54 kernel: [<000000008ac59f4e>]
do_idle+0xf6/0x1b0
Jul 06 17:37:07 t35lp54 kernel: [<000000008ac5a28e>]
cpu_startup_entry+0x36/0x40
Jul 06 17:37:07 t35lp54 kernel: [<000000008abb0d90>]
smp_start_secondary+0x148/0x158
Jul 06 17:37:07 t35lp54 kernel: [<000000008b935b9e>]
restart_int_handler+0x6e/0x90
which confirms that the call is in softirq context. So in_task()
covers exactly
the cases where we want to have CPACF trng called: not in nmi, not in
hard irq,
not in soft irq but in normal task context and during kernel init.
Reluctantly,
Acked-by: Jason A. Donenfeld <Jason@xxxxxxxxx>
I'll let you know if I ever get rid of or optimize the call from
kfence_guarded_alloc() so that maybe there's a chance of reverting
this.
One small unimportant nit:
if (static_branch_likely(&s390_arch_random_available)) {
- cpacf_trng(NULL, 0, (u8 *)v, sizeof(*v));
- atomic64_add(sizeof(*v), &s390_arch_random_counter);
- return true;
+ if (in_task()) {
You can avoid a level of indentation by making this:
if (static_branch_likely(&s390_arch_random_available) && in_task())
But not my code so doesn't really matter to me. So have my Ack above
and
I'll stop being nitpicky :).
Jason
Thanks for your ack. I'll do the improvement you suggested and then push
this patch into the s390 subsystem (with cc: stable).
... and then let's see if we can establish something like
arch_get_random_seed_ bytes() and a way to invoke the trng in interrupt
context without the network guys complaining.
regards
Harald Freudenberger
