Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (1.0)
Subject: Re: [PATCH v3 3/4] uaccess: Check no rescheduling function is called in unsafe region
From:   Andy Lutomirski <luto@amacapital.net>
In-Reply-To: <20190213222146.GC32494@hirez.programming.kicks-ass.net>
Date:   Wed, 13 Feb 2019 14:49:47 -0800
Cc:     Julien Thierry <julien.thierry@arm.com>,
        Will Deacon <will.deacon@arm.com>,
        Ingo Molnar <mingo@kernel.org>, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, mingo@redhat.com,
        catalin.marinas@arm.com, james.morse@arm.com, hpa@zytor.com,
        valentin.schneider@arm.com, brgerst@gmail.com, jpoimboe@redhat.com,
        luto@kernel.org, bp@alien8.de, dvlasenk@redhat.com,
        torvalds@linux-foundation.org, tglx@linutronix.de
Content-Transfer-Encoding: quoted-printable
Message-Id: <D3E1B64D-3E91-4A82-9C3B-92D9EB7326CE@amacapital.net>
References: <a23fdd84-8c4b-0c1f-d877-24d946cff6d0@arm.com> <20190213103553.GO32494@hirez.programming.kicks-ass.net> <1c2429a4-9df9-40a3-98e0-51577de4bd6a@arm.com> <20190213131720.GU32494@hirez.programming.kicks-ass.net> <20190213140025.GB6346@brain-police> <20190213142524.GW32494@hirez.programming.kicks-ass.net> <dd33cc20-f335-0e4c-e3f4-02497ecc3710@arm.com> <20190213144145.GY32494@hirez.programming.kicks-ass.net> <20190213154532.GQ32534@hirez.programming.kicks-ass.net> <D61C430D-4321-4114-AB85-671A3C7B8EAE@amacapital.net> <20190213222146.GC32494@hirez.programming.kicks-ass.net>
To:     Peter Zijlstra <peterz@infradead.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On Feb 13, 2019, at 2:21 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>=20
> On Wed, Feb 13, 2019 at 10:51:24AM -0800, Andy Lutomirski wrote:
>>> On Feb 13, 2019, at 7:45 AM, Peter Zijlstra <peterz@infradead.org> wrote=
:
>=20
>>> Which I suppose means that GCC generates the PUSHF/POPF to preserve the
>>> EFLAGS, since we mark those explicitly clobbered.
>>>=20
>>=20
>> Not quite.  A flags clobber doesn=E2=80=99t save the control bits like AC=

>> except on certain rather buggy llvm compilers. The change you=E2=80=99re
>> looking for is:
>>=20
>> http://git.kernel.org/tip/2c7577a7583747c9b71f26dced7f696b739da745
>=20
> Indeed, failed to find that.
>=20
>>> 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.
>>=20
>> But only explicit scheduling =E2=80=94 preemption and sleepy page faults a=
re
>> fine because the interrupt frame saves flags.
>=20
> No, like pointed out elsewhere in this thread, anything that does
> preempt_disable() is utterly broken with this.
>=20
> Because at that point the IRQ return path doesn't reschedule but
> preempt_enable() will, and that doesn't preserve EFLAGS again.
>=20
>>> 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.
>>=20
>> That=E2=80=99s certainly the easy solution. Or we could teach the might_s=
leep
>> checks about this, but that could be a mess.
>=20
> That's not enough, we'd have to teach preempt_disable(), but worse,
> preempt_disable_notrace().
>=20
> 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 pr=
oblem can=E2=80=99t be too widespread or we would have heard of it before.=