linux-kernel - Re: [PATCH v3 3/4] uaccess: Check no rescheduling function is called in unsafe region

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: <D3E1B64D-3E91-4A82-9C3B-92D9EB7326CE@amacapital.net>
Date:   Wed, 13 Feb 2019 14:49:47 -0800
From:   Andy Lutomirski <luto@...capital.net>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Julien Thierry <julien.thierry@....com>,
        Will Deacon <will.deacon@....com>,
        Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, mingo@...hat.com,
        catalin.marinas@....com, james.morse@....com, hpa@...or.com,
        valentin.schneider@....com, brgerst@...il.com, jpoimboe@...hat.com,
        luto@...nel.org, bp@...en8.de, dvlasenk@...hat.com,
        torvalds@...ux-foundation.org, tglx@...utronix.de
Subject: Re: [PATCH v3 3/4] uaccess: Check no rescheduling function is called in unsafe region



> On Feb 13, 2019, at 2:21 PM, Peter Zijlstra <peterz@...radead.org> wrote:
> 
> On Wed, Feb 13, 2019 at 10:51:24AM -0800, Andy Lutomirski wrote:
>>> On Feb 13, 2019, at 7:45 AM, Peter Zijlstra <peterz@...radead.org> wrote:
> 
>>> Which I suppose means that GCC generates the PUSHF/POPF to preserve the
>>> EFLAGS, since we mark those explicitly clobbered.
>>> 
>> 
>> Not quite.  A flags clobber doesn’t save the control bits like AC
>> except on certain rather buggy llvm compilers. The change you’re
>> looking for is:
>> 
>> http://git.kernel.org/tip/2c7577a7583747c9b71f26dced7f696b739da745
> 
> Indeed, failed to find that.
> 
>>> For a little bit of context; it turns out that user_access_begin() /
>>> user_access_end() sets EFLAGS.AC and scheduling in between there wrecks
>>> that because we're apparently not saving that anymore.
>> 
>> But only explicit scheduling — preemption and sleepy page faults are
>> fine because the interrupt frame saves flags.
> 
> No, like pointed out elsewhere in this thread, anything that does
> preempt_disable() is utterly broken with this.
> 
> Because at that point the IRQ return path doesn't reschedule but
> preempt_enable() will, and that doesn't preserve EFLAGS again.
> 
>>> Now, I'm tempted to add the PUSHF / POPF right back because of this, but
>>> first I suppose we need to figure out if that change was on purpose and
>>> why that went missing from the Changelog.
>> 
>> That’s certainly the easy solution. Or we could teach the might_sleep
>> checks about this, but that could be a mess.
> 
> That's not enough, we'd have to teach preempt_disable(), but worse,
> preempt_disable_notrace().
> 
> Anything that lands in ftrace, which _will_ use
> preempt_disable_notrace(), will screw this thing up.

Ugh.  Consider your patch acked.  Do we need to backport this thing?  The problem can’t be too widespread or we would have heard of it before.