linux-kernel - Re: [PATCH 09/20] signal/vm86_32: Replace open coded BUG_ON with an actual BUG

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <87ilwxrvu9.fsf@email.froward.int.ebiederm.org>
Date:   Fri, 12 Nov 2021 13:57:02 -0600
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Brian Gerst <brgerst@...il.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.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>,
        Andy Lutomirski <luto@...nel.org>
Subject: Re: [PATCH 09/20] signal/vm86_32: Replace open coded BUG_ON with an actual BUG_ON

Brian Gerst <brgerst@...il.com> writes:

> On Fri, Nov 12, 2021 at 10:41 AM Eric W. Biederman
> <ebiederm@...ssion.com> wrote:
>>
>> "Eric W. Biederman" <ebiederm@...ssion.com> writes:
>>
>> > The function save_v86_state is only called when userspace was
>> > operating in vm86 mode before entering the kernel.  Not having vm86
>> > state in the task_struct should never happen.  So transform the hand
>> > rolled BUG_ON into an actual BUG_ON to make it clear what is
>> > happening.
>>
>> Now that this change has been merged into Linus' tree I have a report
>> that it is possible to trigger this new BUG_ON.  Which obviously is not
>> good.
>>
>> We could revert the change but I think that would just be shooting the
>> messenger.
>>
>> Does anyone have an idea where to start to track down what is going on?
>>
>> A very quick skim through the code suggests that the only code path
>> that calls save_v86_state that has not already accessed is
>> current->thread.vm86 is handle_signal.
>>
>> Another quick look suggests that the only place where X86_VM_MASK gets
>> set in eflags is in do_sys_vm86.  So it appears do_sys_vm86 must
>> be called before v8086_mode returns true in handle_signal.
>>
>> Which seems to suggest that the bug on can't trigger.
>>
>> But that is obviously wrong.
>>
>> I will keep digging but if anyone has some ideas that would be appreciated.
>>
>> Eric
>
> It's possible that a null pointer was passed to the vm86 syscall.
> Since vm86 mode usually requires memory to be mapped at address 0 this
> wouldn't trigger a fault when reading the vm86_struct data.  It should
> be fine to remove !vm86->user_vm86 from the BUG_ON(), since the write
> to userspace can handle a fault.

Agreed, and that is plausible.

Still user space would have had to have mapped address 0 to get that
value set in do_sys_vm86.

I will see about cooking up a patch for that case regardless.  I am not
quite convinced, but perhaps the easiest way to tell is to simply remove
the unnecessary test and ask the fuzzer folks to see if they can still
hit the BUG_ON.

Eric