Re: [PATCH] RISC-V: KVM: Fix TOCTOU race in SBI system suspend handler

[Jiakai Xu]

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