Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled
From: Halil Pasic
Date: Wed Sep 27 2023 - 13:12:23 EST
On Tue, 26 Sep 2023 18:41:58 +0200
Halil Pasic <pasic@xxxxxxxxxxxxx> wrote:
> > + local_bh_disable();
> > crypto_finalize_akcipher_request(vc_akcipher_req->base.dataq->engine, req, err);
> > + local_bh_enable();
>
> Thanks Gonglei!
>
> I did this a quick spin, and it does not seem to be sufficient on s390x.
> Which does not come as a surprise to me, because
>
> #define lockdep_assert_in_softirq() \
> do { \
> WARN_ON_ONCE(__lockdep_enabled && \
> (!in_softirq() || in_irq() || in_nmi())); \
> } while (0)
>
> will still warn because in_irq() still evaluates to true (your patch
> addresses the !in_softirq() part).
>
> I don't have any results on x86 yet. My current understanding is that the
> virtio-pci transport code disables interrupts locally somewhere in the
> call chain (actually in vp_vring_interrupt() via spin_lock_irqsave())
> and then x86 would be fine. But I will get that verified.
[ 35.177962][ C0] WARNING: CPU: 0 PID: 152 at kernel/softirq.c:306 __local_bh_disable_ip (kernel/softirq.c:306 (discriminator 1))
[ 35.178551][ C0] Modules linked in: vmw_vsock_virtio_transport(+) vmw_vsock_virtio_transport_common virtio_crypto(+) crypto_engine vsock
[ 35.179930][ C0] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc38 04/01/2014
[ 35.180548][ C0] RIP: 0010:__local_bh_disable_ip (kernel/softirq.c:306 (discriminator 1))
[ 35.180936][ C0] Code: eb 7d 65 8b 05 ef 90 eb 7d 31 f0 f6 c4 ff 74 13 9c 58 f6 c4 02 75 17 80 e7 02 74 01 fb 5b c3 cc cc cc cc e8 48 2f 15 00 eb e6 <0f> 0b eb ca e8 2d 88 03 03 eb e2 66 66 2e 0f 1f 84 00 00 00 00 00
All code
========
0: eb 7d jmp 0x7f
2: 65 8b 05 ef 90 eb 7d mov %gs:0x7deb90ef(%rip),%eax # 0x7deb90f8
9: 31 f0 xor %esi,%eax
b: f6 c4 ff test $0xff,%ah
e: 74 13 je 0x23
10: 9c pushf
11: 58 pop %rax
12: f6 c4 02 test $0x2,%ah
15: 75 17 jne 0x2e
17: 80 e7 02 and $0x2,%bh
1a: 74 01 je 0x1d
1c: fb sti
1d: 5b pop %rbx
1e: c3 ret
1f: cc int3
20: cc int3
21: cc int3
22: cc int3
23: e8 48 2f 15 00 call 0x152f70
28: eb e6 jmp 0x10
2a:* 0f 0b ud2 <-- trapping instruction
2c: eb ca jmp 0xfffffffffffffff8
2e: e8 2d 88 03 03 call 0x3038860
33: eb e2 jmp 0x17
35: 66 66 2e 0f 1f 84 00 data16 cs nopw 0x0(%rax,%rax,1)
3c: 00 00 00 00
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: eb ca jmp 0xffffffffffffffce
4: e8 2d 88 03 03 call 0x3038836
9: eb e2 jmp 0xffffffffffffffed
b: 66 66 2e 0f 1f 84 00 data16 cs nopw 0x0(%rax,%rax,1)
12: 00 00 00 00
[ 35.182237][ C0] RSP: 0018:ffffc90000007d88 EFLAGS: 00010006
[ 35.182637][ C0] RAX: 0000000080010003 RBX: ffff888108308538 RCX: ffffc90000007d50
[ 35.183186][ C0] RDX: ffff88811ae36300 RSI: 0000000000000200 RDI: ffffffffc02b16cc
[ 35.183700][ C0] RBP: ffff8881083084e8 R08: 0000000000000000 R09: fffffbfff0d04f04
[ 35.184216][ C0] R10: ffffffff86827823 R11: ffffffff852013e6 R12: 0000000000000001
[ 35.184730][ C0] R13: 0000000000000000 R14: ffff888108308538 R15: dffffc0000000000
[ 35.185248][ C0] FS: 00007f06cb551800(0000) GS:ffff88811ae00000(0000) knlGS:0000000000000000
[ 35.185831][ C0] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 35.186271][ C0] CR2: 000055dc93010628 CR3: 0000000116b28000 CR4: 00000000000006f0
[ 35.186789][ C0] Call Trace:
[ 35.187010][ C0] <IRQ>
[ 35.187204][ C0] ? __warn (kernel/panic.c:673)
[ 35.187505][ C0] ? __local_bh_disable_ip (kernel/softirq.c:306 (discriminator 1))
[ 35.187857][ C0] ? report_bug (lib/bug.c:180 lib/bug.c:219)
[ 35.188197][ C0] ? handle_bug (arch/x86/kernel/traps.c:237 (discriminator 1))
[ 35.188483][ C0] ? exc_invalid_op (arch/x86/kernel/traps.c:258 (discriminator 1))
[ 35.188790][ C0] ? asm_exc_invalid_op (./arch/x86/include/asm/idtentry.h:568)
[ 35.189120][ C0] ? asm_common_interrupt (./arch/x86/include/asm/idtentry.h:636)
[ 35.189466][ C0] ? virtio_crypto_dataq_sym_callback (drivers/crypto/virtio/virtio_crypto_skcipher_algs.c:567 drivers/crypto/virtio/virtio_crypto_skcipher_algs.c:81 drivers/crypto/virtio/virtio_crypto_skcipher_algs.c:55) virtio_crypto
[ 35.189983][ C0] ? __local_bh_disable_ip (kernel/softirq.c:306 (discriminator 1))
[ 35.190336][ C0] virtio_crypto_dataq_sym_callback (drivers/crypto/virtio/virtio_crypto_skcipher_algs.c:570 drivers/crypto/virtio/virtio_crypto_skcipher_algs.c:81 drivers/crypto/virtio/virtio_crypto_skcipher_algs.c:55) virtio_crypto
[ 35.190837][ C0] virtcrypto_dataq_callback (drivers/crypto/virtio/virtio_crypto_core.c:91) virtio_crypto
[ 35.191304][ C0] ? __pfx_virtcrypto_dataq_callback (drivers/crypto/virtio/virtio_crypto_core.c:76) virtio_crypto
[ 35.191796][ C0] ? __pfx_do_raw_spin_lock (kernel/locking/spinlock_debug.c:113)
[ 35.192154][ C0] vring_interrupt (drivers/virtio/virtio_ring.c:2598)
[ 35.192536][ C0] vp_vring_interrupt (drivers/virtio/virtio_pci_common.c:67 (discriminator 2))
[ 35.193064][ C0] ? __pfx_vp_vring_interrupt (drivers/virtio/virtio_pci_common.c:60)
[ 35.193793][ C0] __handle_irq_event_percpu (kernel/irq/handle.c:158)
[ 35.194272][ C0] handle_irq_event (kernel/irq/handle.c:195 kernel/irq/handle.c:210)
[ 35.194587][ C0] handle_edge_irq (kernel/irq/chip.c:833)
[ 35.194903][ C0] __common_interrupt (arch/x86/kernel/irq.c:271)
[ 35.195232][ C0] common_interrupt (arch/x86/kernel/irq.c:247 (discriminator 47))
So I was wrong, this patch is not sufficient, not on x86 nor on s390x.
And the problem is that we are in hardirq context.
For some reason, I was under the impression that disabling interrupts in
a hardirq context somehow takes you out of hardirq context and makes
in_irq() return false. Silly me! (I was assuming the fix works on x86 and
hallucinated based on that assumption and any differences I have found
between virtio-ccw and virtio-pci.)
Currently I don't see a need to fix anything in virtio-ccw.
Regards,
Halil