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] x86: uaccess: fix regression in unsafe_get_user
From:   Andy Lutomirski <luto@amacapital.net>
In-Reply-To: <20190216234702.GP2217@ZenIV.linux.org.uk>
Date:   Sat, 16 Feb 2019 18:36:52 -0800
Cc:     Thomas Gleixner <tglx@linutronix.de>, Jann Horn <jannh@google.com>,
        baloo@gandi.net, the arch/x86 maintainers <x86@kernel.org>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        kernel list <linux-kernel@vger.kernel.org>,
        Pascal Bouchareine <pascal@gandi.net>
Content-Transfer-Encoding: quoted-printable
Message-Id: <FBED20DC-1F47-4038-81CC-8F5119785F8E@amacapital.net>
References: <20190215235901.23541-1-baloo@gandi.net> <CAG48ez2tYizTKncevLF=AMQ2nm3D=SqGHH5bM5f-U0fhQ1nL9Q@mail.gmail.com> <alpine.DEB.2.21.1902161358160.1683@nanos.tec.linutronix.de> <4F2693EA-1553-4F09-9475-781305540DBC@amacapital.net> <20190216234702.GP2217@ZenIV.linux.org.uk>
To:     Al Viro <viro@zeniv.linux.org.uk>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On Feb 16, 2019, at 3:47 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>=20
>> On Sat, Feb 16, 2019 at 02:50:15PM -0800, Andy Lutomirski wrote:
>>=20
>> What is the actual problem?  We=E2=80=99re not actually demand-faulting t=
his data, are we?  Are we just overrunning the buffer because the from_user h=
elpers are too clever?  Can we fix it for real by having the fancy helpers d=
o *aligned* loads so that they don=E2=80=99t overrun the buffer?  Heck, this=
 might be faster, too.
>=20
> Unaligned _stores_ are not any cheaper, and you'd get one hell of
> extra arithmetics from trying to avoid both.  Check something
> like e.g. memcpy() on alpha, where you really have to keep all
> accesses aligned, both on load and on store side.

I think we should avoid unaligned loads and do unaligned stores instead.

I would general expect that unaligned stores are a bit cheaper since they do=
n=E2=80=99t need to complete for the comparisons to happen.

But I maintain my claim that this code should not be overrunning its input b=
uffer into the next page, since it could have observable side effects.

>=20
> Can't we just pad the buffers a bit?  Making sure that name_buf
> and symlink_buf are _not_ followed by unmapped pages shouldn't
> be hard.  Both are allocated by kmalloc(), so...
>=20
> What am I missing here?