Received-SPF: pass (google.com: domain of linux-kernel+bounces-99599-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
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: <3e180c07-53db-4acb-a75c-1a33447d81af@app.fastmail.com>
In-Reply-To: <20240311164638.2015063-12-pasha.tatashin@soleen.com>
References: <20240311164638.2015063-1-pasha.tatashin@soleen.com>
 <20240311164638.2015063-12-pasha.tatashin@soleen.com>
Date: Mon, 11 Mar 2024 15:17:28 -0700
From: "Andy Lutomirski" <luto@kernel.org>
To: "Pasha Tatashin" <pasha.tatashin@soleen.com>,
 "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
 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
Content-Transfer-Encoding: quoted-printable


On Mon, Mar 11, 2024, at 9:46 AM, Pasha Tatashin wrote:
> Add dynamic_stack_fault() calls to the kernel faults, and also declare
> HAVE_ARCH_DYNAMIC_STACK =3D y, so that dynamic kernel stacks can be
> enabled on x86 architecture.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
>  arch/x86/Kconfig        | 1 +
>  arch/x86/kernel/traps.c | 3 +++
>  arch/x86/mm/fault.c     | 3 +++
>  3 files changed, 7 insertions(+)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 5edec175b9bf..9bb0da3110fa 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -197,6 +197,7 @@ config X86
>  	select HAVE_ARCH_USERFAULTFD_WP         if X86_64 && USERFAULTFD
>  	select HAVE_ARCH_USERFAULTFD_MINOR	if X86_64 && USERFAULTFD
>  	select HAVE_ARCH_VMAP_STACK		if X86_64
> +	select HAVE_ARCH_DYNAMIC_STACK		if X86_64
>  	select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET
>  	select HAVE_ARCH_WITHIN_STACK_FRAMES
>  	select HAVE_ASM_MODVERSIONS
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index c3b2f863acf0..cc05401e729f 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -413,6 +413,9 @@ DEFINE_IDTENTRY_DF(exc_double_fault)
>  	}
>  #endif
>=20
> +	if (dynamic_stack_fault(current, address))
> +		return;
> +

Sorry, but no, you can't necessarily do this.  I say this as the person =
who write this code, and I justified my code on the basis that we are no=
t recovering -- we're jumping out to a different context, and we won't c=
rash if the origin context for the fault is corrupt.  The SDM is really =
quite unambiguous about it: we're in an "abort" context, and returning i=
s not allowed.  And I this may well be is the real deal -- the microcode=
 does not promise to have the return frame and the actual faulting conte=
xt matched up here, and there's is no architectural guarantee that retur=
ning will do the right thing.

Now we do have some history of getting a special exception, e.g. for esp=
fix64.  But espfix64 is a very special case, and the situation you're lo=
oking at is very general.  So unless Intel and AMD are both wiling to pu=
blicly document that it's okay to handle stack overflow, where any instr=
uction in the ISA may have caused the overflow, like this, then we're no=
t going to do it.

There are some other options: you could pre-map=20

Also, I think the whole memory allocation concept in this whole series i=
s a bit odd.  Fundamentally, we *can't* block on these stack faults -- w=
e may be in a context where blocking will deadlock.  We may be in the pa=
ge allocator.  Panicing due to kernel stack allocation would be very unp=
leasant.  But perhaps we could have a rule that a task can only be sched=
uled in if there is sufficient memory available for its stack.  And perh=
aps we could avoid every page-faulting by filling in the PTEs for the po=
tential stack pages but leaving them un-accessed.  I *think* that all x8=
6 implementations won't fill the TLB for a non-accessed page without als=
o setting the accessed bit, so the performance hit of filling the PTEs, =
running the task, and then doing the appropriate synchronization to clea=
r the PTEs and read the accessed bit on schedule-out to release the page=
s may not be too bad.  But you would need to do this cautiously in the s=
cheduler, possibly in the *next* task but before the prev task is actual=
ly released enough to be run on a different CPU.  It's going to be messy=
.