Received-SPF: pass (google.com: domain of linux-kernel+bounces-99684-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Feedback-ID: ieff94742:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-251-g8332da0bf6-fm-20240305.001-g8332da0b
Precedence: bulk
MIME-Version: 1.0
Message-Id: <11e673cf-8bfa-4493-ab86-2f1f97ddd732@app.fastmail.com>
In-Reply-To: <08EFDEDB-7BBB-4D9C-B7E5-D7370EC609BE@gmail.com>
References: <20240311164638.2015063-1-pasha.tatashin@soleen.com>
 <20240311164638.2015063-12-pasha.tatashin@soleen.com>
 <3e180c07-53db-4acb-a75c-1a33447d81af@app.fastmail.com>
 <bedfa55b-b1d0-4e59-8c94-dbc5f8485a7f@intel.com>
 <ef0419dd-9d7b-4b77-b63a-5f11aaefb570@app.fastmail.com>
 <08EFDEDB-7BBB-4D9C-B7E5-D7370EC609BE@gmail.com>
Date: Mon, 11 Mar 2024 17:02:40 -0700
From: "Andy Lutomirski" <luto@kernel.org>
To: "Nadav Amit" <nadav.amit@gmail.com>
Cc: "Dave Hansen" <dave.hansen@intel.com>,
 "Pasha Tatashin" <pasha.tatashin@soleen.com>,
 "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
 linux-mm <linux-mm@kvack.org>,
 "Andrew Morton" <akpm@linux-foundation.org>,
 "the arch/x86 maintainers" <x86@kernel.org>,
 "Borislav Petkov" <bp@alien8.de>,
 "Christian Brauner" <brauner@kernel.org>, bristot@redhat.com,
 "Ben Segall" <bsegall@google.com>,
 "Dave Hansen" <dave.hansen@linux.intel.com>, dianders@chromium.org,
 dietmar.eggemann@arm.com, eric.devolder@oracle.com, hca@linux.ibm.com,
 "hch@infradead.org" <hch@infradead.org>,
 "H. Peter Anvin" <hpa@zytor.com>,
 "Jacob Pan" <jacob.jun.pan@linux.intel.com>,
 "Jason Gunthorpe" <jgg@ziepe.ca>, jpoimboe@kernel.org,
 "Joerg Roedel" <jroedel@suse.de>, juri.lelli@redhat.com,
 "Kent Overstreet" <kent.overstreet@linux.dev>, kinseyho@google.com,
 "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
 lstoakes@gmail.com, mgorman@suse.de, mic@digikod.net,
 michael.christie@oracle.com, "Ingo Molnar" <mingo@redhat.com>,
 mjguzik@gmail.com, "Michael S. Tsirkin" <mst@redhat.com>,
 "Nicholas Piggin" <npiggin@gmail.com>,
 "Peter Zijlstra (Intel)" <peterz@infradead.org>,
 "Petr Mladek" <pmladek@suse.com>,
 "Rick P Edgecombe" <rick.p.edgecombe@intel.com>,
 "Steven Rostedt" <rostedt@goodmis.org>,
 "Suren Baghdasaryan" <surenb@google.com>,
 "Thomas Gleixner" <tglx@linutronix.de>,
 "Uladzislau Rezki" <urezki@gmail.com>, vincent.guittot@linaro.org,
 vschneid@redhat.com
Subject: Re: [RFC 11/14] x86: add support for Dynamic Kernel Stacks
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable


On Mon, Mar 11, 2024, at 4:56 PM, Nadav Amit wrote:
>> On 12 Mar 2024, at 1:41, Andy Lutomirski <luto@kernel.org> wrote:
>>=20
>> On Mon, Mar 11, 2024, at 4:34 PM, Dave Hansen wrote:
>>> On 3/11/24 15:17, Andy Lutomirski wrote:
>>>> I *think* that all x86 implementations won't fill the TLB for a
>>>> non-accessed page without also setting the accessed bit,
>>>=20
>>> That's my understanding as well.  The SDM is a little more obtuse ab=
out it:
>>>=20
>>>> Whenever the processor uses a paging-structure entry as part of
>>>> linear-address translation, it sets the accessed flag in that entry
>>>> (if it is not already set).
>>>=20
>>> but it's there.
>>>=20
>>> But if we start needing Accessed=3D1 to be accurate, clearing those =
PTEs
>>> gets more expensive because it needs to be atomic to lock out the pa=
ge
>>> walker.  It basically needs to start getting treated similarly to wh=
at
>>> is done for Dirty=3D1 on userspace PTEs.  Not the end of the world, =
of
>>> course, but one more source of overhead.
>>=20
>> In my fantasy land where I understand the x86 paging machinery, suppo=
se we're in finish_task_switch(), and suppose prev is Not Horribly Buggy=
 (TM).  In particular, suppose that no other CPU is concurrently (non-sp=
eculatively!) accessing prev's stack.  Prev can't be running, because wh=
atever magic lock prevents it from being migrated hasn't been released y=
et.  (I have no idea what lock this is, but it had darned well better ex=
ist so prev isn't migrated before switch_to() even returns.)
>>=20
>> So the current CPU is not accessing the memory, and no other CPU is a=
ccessing the memory, and BPF doesn't exist, so no one is being utterly d=
aft and a kernel read probe, and perf isn't up to any funny business, et=
c.  And a CPU will never *speculatively* set the accessed bit (I told yo=
u it's fantasy land), so we just do it unlocked:
>>=20
>> if (!pte->accessed) {
>>  *pte =3D 0;
>>  reuse the memory;
>> }
>>=20
>> What could possibly go wrong?
>>=20
>> I admit this is not the best idea I've ever had, and I will not waste=
 anyone's time by trying very hard to defend it :)
>>=20
>
> Just a thought: you don=E2=80=99t care if someone only reads from the =
stack's=20
> page (you can just install another page later). IOW: you only care if=20
> someone writes.
>
> So you can look on the dirty-bit, which is not being set speculatively=20
> and save yourself one problem.

Doesn't this buy a new problem?  Install a page, run the thread without =
using the page but speculatively load the PTE as read-only into the TLB,=
 context-switch out the thread, (entirely safely and correctly) determin=
e that the page wasn't used, remove it from the PTE, use it for somethin=
g else and fill it with things that aren't zero, run the thread again, a=
nd read from it.  Now it has some other thread's data!

One might slightly credibly argue that this isn't a problem -- between R=
SP and the bottom of the area that one nominally considers to the by the=
 stack is allowed to return arbitrary garbage, especially in the kernel =
where there's no red zone (until someone builds a kernel with a redzone =
on a FRED system, hmm), but this is still really weird.  If you *write* =
in that area, the CPU hopefully puts the *correct* value in the TLB and =
life goes on, but how much do you trust anyone to have validated what ha=
ppens when a PTE is present, writable and clean but the TLB contains a s=
tale entry pointing somewhere else?  And is it really okay to do this to=
 the poor kernel?

If we're going to add a TLB flush on context switch, then (a) we are bei=
ng rather silly and (b) we might as well just use atomics to play with t=
he accessed bit instead, I think.