Re: [PATCH] RISC-V: KVM: Fix TOCTOU race in SBI system suspend handler
Jiakai Xu <[email protected]>
| Newsgroups | org.infradead.lists.kvm-riscv,org.infradead.lists.linux-riscv,org.kernel.vger.kvm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Hi, drew! Thanks for your review! > > I'm not a big fan of this approach and I see sashiko found it has gaps[1]. I read sashiko's feedback and I think it makes sense. > > I'd rather we introduce a mutex to kvm_arch to serialize cross-vcpu > > Eh, not sure why I said 'introduce' here. We can just use kvm->lock. I looked into that, but it may be not suitable. Here's why: Documentation/virt/kvm/locking.rst explicitly states: kvm->lock is taken outside vcpu->mutex. The susp handler runs inside kvm_arch_vcpu_ioctl_run(), which is called from kvm_vcpu_ioctl() where vcpu->mutex is already held. Taking kvm->lock inside the susp handler would produce: vcpu->mutex → kvm->lock. So may be we should introduce a mutex to kvm_arch to serialize cross-vcpu mp-state operations. What do you think? > > mp-state operations. So it'd be taken in SUSP, HART_START, and also SRST > > And, even though SRST doesn't really need it - why not... Consistency has > its merits. > > > (anywhere that reads or modifies another vcpu's mp-state). Operations that > > only modify the calling vcpu's own state would still only use the per-vcpu > > mp_state_lock. > > > > [1] https://sashiko.dev/#/patchset/20260521142030.1560861-1-xujiakai2025%40iscas.ac.cn I agree with you. Regards, Jiakai -- kvm-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kvm-riscv