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 and patience in discussing this issue with me. > > > But... before we get too carried away. Unless I'm missing something, this > > > race really looks too theoretical to justify any change at all. Do you > > > have a test case in mind that could force it? > > > > Unfortunately, I don't have a reproducer or test case that can reliably > > trigger this race. > > > > But as we saw earlier, this is a real bug, and we can tell that just by > > analyzing the code. > > > > The real environment is complex and unpredictable, and such races are > > possible. Wouldn't it be nice if we found it and fixed it now? > > > > I assume the scenario we're trying to fix is: > > HART-x HART-z > ------ ------ > SUSP() > Check HART-y is stopped > HART_START(HART-y) > HART_STOP() > Check HART-z is stopped > Determine all HARTs are stopped and exit to userspace Yes, this is exactly the scenario we discussed earlier. > > Besides the fact that the race seems quite unlikely (only a pathological > guest would try to force it), userspace can/should still double check that > all HARTs are stopped before proceeding with its system suspend > implementation. So, since we would need to introduce a new lock to resolve > this very unlikely issue, then I'm not 100% convinced we should. I would > be more willing to fix it if I thought the host was at any risk, but I > think this is "only" a guest corruption issue. If the guest does wild > things like this during a system suspend, then it can keep all its pieces. > I would agree that we should add a comment about this choice, though. I agree with you. I'll send a v2 that replaces the fix with a comment documenting the race and our decision. Thanks for the thorough review again! Regards, Jiakai -- kvm-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kvm-riscv