linux-kernel - Re: [PATCH v2 10/32] signal/vm86_32: Properly send SIGSEGV when the vm86 state cannot be saved.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <530952e5-27d7-40b8-ac9a-debc36bb4fdf@www.fastmail.com>
Date:   Mon, 25 Oct 2021 17:21:36 -0700
From:   "Andy Lutomirski" <luto@...nel.org>
To:     "Linus Torvalds" <torvalds@...ux-foundation.org>
Cc:     "Eric W. Biederman" <ebiederm@...ssion.com>,
        "Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        "Oleg Nesterov" <oleg@...hat.com>,
        "Al Viro" <viro@...iv.linux.org.uk>,
        "Kees Cook" <keescook@...omium.org>,
        "Thomas Gleixner" <tglx@...utronix.de>,
        "Ingo Molnar" <mingo@...hat.com>, "Borislav Petkov" <bp@...en8.de>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH v2 10/32] signal/vm86_32: Properly send SIGSEGV when the vm86 state
 cannot be saved.



On Mon, Oct 25, 2021, at 4:45 PM, Linus Torvalds wrote:
> On Mon, Oct 25, 2021 at 3:25 PM Andy Lutomirski <luto@...nel.org> 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(&regs->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