Re: [RFC 11/14] x86: add support for Dynamic Kernel Stacks

From: Thomas Gleixner <tglx@linutronix.de>
To: Pasha Tatashin <pasha.tatashin@soleen.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	akpm@linux-foundation.org, x86@kernel.org, bp@alien8.de,
	brauner@kernel.org, bristot@redhat.com, bsegall@google.com,
	dave.hansen@linux.intel.com, dianders@chromium.org,
	dietmar.eggemann@arm.com, eric.devolder@oracle.com,
	hca@linux.ibm.com, hch@infradead.org, hpa@zytor.com,
	jacob.jun.pan@linux.intel.com, jgg@ziepe.ca, jpoimboe@kernel.org,
	jroedel@suse.de, juri.lelli@redhat.com,
	kent.overstreet@linux.dev, kinseyho@google.com,
	kirill.shutemov@linux.intel.com, lstoakes@gmail.com,
	luto@kernel.org, mgorman@suse.de, mic@digikod.net,
	michael.christie@oracle.com, mingo@redhat.com, mjguzik@gmail.com,
	mst@redhat.com, npiggin@gmail.com, peterz@infradead.org,
	pmladek@suse.com, rick.p.edgecombe@intel.com,
	rostedt@goodmis.org, surenb@google.com, urezki@gmail.com,
	vincent.guittot@linaro.org, vschneid@redhat.com
Subject: Re: [RFC 11/14] x86: add support for Dynamic Kernel Stacks
Date: Wed, 13 Mar 2024 17:12:22 +0100	[thread overview]
Message-ID: <87bk7inmah.ffs@tglx> (raw)
In-Reply-To: <CA+CK2bAzJuCe06g_TEOh3B-hK+dXfUaGaOSTgzyxkN4zqpSU_A@mail.gmail.com>

On Wed, Mar 13 2024 at 11:28, Pasha Tatashin wrote:
> On Wed, Mar 13, 2024 at 9:43 AM Pasha Tatashin
> <pasha.tatashin@soleen.com> wrote:
>> Here's a potential solution that is fast, avoids locking, and ensures atomicity:
>>
>> 1. Kernel Stack VA Space
>> Dedicate a virtual address range ([KSTACK_START_VA - KSTACK_END_VA])
>> exclusively for kernel stacks. This simplifies validation of faulting
>> addresses to be part of a stack.
>>
>> 2. Finding the faulty task
>> - Use ALIGN(fault_address, THREAD_SIZE) to calculate the end of the
>> topmost stack page (since stack addresses are aligned to THREAD_SIZE).
>> - Store the task_struct pointer as the last word on this topmost page,
>> that is always present as it is a pre-allcated stack page.
>>
>> 3. Stack Padding
>> Increase padding to 8 bytes on x86_64 (TOP_OF_KERNEL_STACK_PADDING 8)
>> to accommodate the task_struct pointer.
>
> Alternatively, do not even look-up the task_struct in
> dynamic_stack_fault(), but only install the mapping to the faulting
> address, store va in the per-cpu array, and handle the rest in
> dynamic_stack() during context switching. At that time spin locks can
> be taken, and we can do a find_vm_area(addr) call.
>
> This way, we would not need to modify TOP_OF_KERNEL_STACK_PADDING to
> keep task_struct in there.

Why not simply doing the 'current' update right next to the stack
switching in __switch_to_asm() which has no way of faulting.

That needs to validate whether anything uses current between the stack
switch and the place where current is updated today. I think nothing
should do so, but I would not be surprised either if it would be the
case. Such code would already today just work by chance I think,

That should not be hard to analyze and fixup if necessary.

So that's fixable, but I'm not really convinced that all of this is safe
and correct under all circumstances. That needs a lot more analysis than
just the trivial one I did for switch_to().

Thanks,

        tglx