Received-SPF: pass (google.com: domain of linux-kernel+bounces-112875-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-332-gdeb4194079-fm-20240319.002-gdeb41940
Precedence: bulk
MIME-Version: 1.0
Message-Id: <b0a07878-a9f1-40aa-b177-423b05137d2e@app.fastmail.com>
In-Reply-To: <b5624bba-9917-421b-8ef0-4515d442f80b@ghiti.fr>
References: <20240313180010.295747-1-samuel.holland@sifive.com>
 <CAHVXubjLWZkjSapnsWJzimWg_2swEy7tQ-DQ6ri8yMk8-Qsc-A@mail.gmail.com>
 <88de4a1a-047e-4be9-b5b0-3e53434dc022@sifive.com>
 <b5624bba-9917-421b-8ef0-4515d442f80b@ghiti.fr>
Date: Sun, 24 Mar 2024 23:05:52 +0100
From: "Arnd Bergmann" <arnd@arndb.de>
To: "Alexandre Ghiti" <alex@ghiti.fr>,
 "Samuel Holland" <samuel.holland@sifive.com>,
 "Alexandre Ghiti" <alexghiti@rivosinc.com>
Cc: "Palmer Dabbelt" <palmer@dabbelt.com>, linux-riscv@lists.infradead.org,
 "Albert Ou" <aou@eecs.berkeley.edu>,
 "Andrew Morton" <akpm@linux-foundation.org>,
 "Charlie Jenkins" <charlie@rivosinc.com>, guoren <guoren@kernel.org>,
 "Jisheng Zhang" <jszhang@kernel.org>,
 "Kemeng Shi" <shikemeng@huaweicloud.com>,
 "Matthew Wilcox" <willy@infradead.org>, "Mike Rapoport" <rppt@kernel.org>,
 "Paul Walmsley" <paul.walmsley@sifive.com>,
 "Xiao W Wang" <xiao.w.wang@intel.com>, "Yangyu Chen" <cyy@cyyself.name>,
 linux-kernel@vger.kernel.org
Subject: Re: [PATCH] riscv: Define TASK_SIZE_MAX for __access_ok()
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable

On Tue, Mar 19, 2024, at 17:51, Alexandre Ghiti wrote:
> On 18/03/2024 22:29, Samuel Holland wrote:
>> On 2024-03-18 3:50 PM, Alexandre Ghiti wrote:
>>> On Wed, Mar 13, 2024 at 7:00=E2=80=AFPM Samuel Holland
>> It looks like the call to fixup_exception() [added
>> in 416721ff05fd ("riscv, mm: Perform BPF exhandler fixup on page faul=
t")] is
>> only intended to catch null pointer dereferences. So making the chang=
e wouldn't
>> have any functional impact, but it would still be a valid optimizatio=
n.
>>
>>> Or I was wondering if it would not be better to do like x86 and use =
an
>>> alternative, it would be more correct (even though I believe your
>>> solution works)
>>> https://elixir.bootlin.com/linux/latest/source/arch/x86/include/asm/=
page_64.h#L82.
>> What would be the benefit of using an alternative? Any access to an a=
ddress
>> between TASK_SIZE and TASK_SIZE_MAX is guaranteed to generate a page =
fault, so
>> the only benefit I see is returning -EFAULT slightly faster at the co=
st of
>> applying a few hundred alternatives at boot. But it's possible I'm mi=
ssing
>> something.
>
>
> The use of alternatives allows to return right away if the buffer is=20
> beyond the usable user address space, and it's not just "slightly=20
> faster" for some cases (a very large buffer with only a few bytes bein=
g=20
> beyond the limit or someone could fault-in all the user pages and fail=20
> very late...etc). access_ok() is here to guarantee that such situation=
s=20
> don't happen, so actually it makes more sense to use an alternative to=20
> avoid that.

The access_ok() function really wants a compile-time constant
value for TASK_SIZE_MAX so it can do constant folding for
repeated calls inside of one function, so for configurations
with a boot-time selected TASK_SIZE_64 it's already not ideal,
with or without alternatives.

If I read the current code correctly, riscv doesn't even
have a way to build with a compile-time selected
VA_BITS/PGDIR_SIZE, which is probably a better place to
start optimizing, since this rarely needs to be selected
dynamically.

      Arnd