On Mon, Oct 25, 2021, at 4:45 PM, Linus Torvalds wrote: > On Mon, Oct 25, 2021 at 3:25 PM Andy Lutomirski <luto@xxxxxxxxxx> wrote: >> >> I think the result would be nicer if, instead of adding an extra goto, >> you just literally moved all the cleanup under the unsafe_put_user()s >> above them. Unless I missed something, none of the put_user stuff reads >> any state that is written by the cleanup code. > > Sure it does: > > memcpy(®s->pt, &vm86->regs32, sizeof(struct pt_regs)); > > is very much part of the cleanup code, and overwrites that regs->pt thing. > > Which is exactly what we're writing back to user space in that > unsafe_put_user() thing. D’oh, right. > > That said, thinking more about this, and looking at it again, I take > back my statement that we could just make it a catchable SIGSEGV > instead. > > If we can't write the vm86 state to user space, we will have > fundamentally lost it, and while it's not fatal to the kernel, and > while we've recovered the original 32-bit state, it's not something > that user space can sanely recover from because the register state at > the end of the vm86 work has now been irrecoverably thrown away. There’s “recoverable” and there’s “recoverable”. Sure, the vm86 state is gone, but the process is getting a signal that doesn’t indicate that one can freely return and carry on as if nothing happened. But one can catch the signal and go on to do something else. > > So I think Eric's patch is fine. Me too. > > Except, as mentioned as part of the other patch, the "force_sigsegv()" > conversion to use "force_fatal_sig()" was broken, because that > function wasn't actually fatal at all. > > Linus
