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.
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.
So I think Eric's patch is fine.
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
