Re: [PATCH RFC v2 02/29] x86: Create CONFIG_MITIGATION_ADDRESS_SPACE_ISOLATION

[Mike Rapoport <rppt@kernel.org>]

Hi Brendan,

On Fri, Jan 10, 2025 at 06:40:28PM +0000, Brendan Jackman wrote:
> Currently a nop config. Keeping as a separate commit for easy review of
> the boring bits. Later commits will use and enable this new config.
> 
> This config is only added for non-UML x86_64 as other architectures do
> not yet have pending implementations. It also has somewhat artificial
> dependencies on !PARAVIRT and !KASAN which are explained in the Kconfig
> file.
> 
> Co-developed-by: Junaid Shahid <junaids@google.com>
> Signed-off-by: Junaid Shahid <junaids@google.com>
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---
>  arch/alpha/include/asm/Kbuild      |  1 +
>  arch/arc/include/asm/Kbuild        |  1 +
>  arch/arm/include/asm/Kbuild        |  1 +
>  arch/arm64/include/asm/Kbuild      |  1 +
>  arch/csky/include/asm/Kbuild       |  1 +
>  arch/hexagon/include/asm/Kbuild    |  1 +
>  arch/loongarch/include/asm/Kbuild  |  3 +++
>  arch/m68k/include/asm/Kbuild       |  1 +
>  arch/microblaze/include/asm/Kbuild |  1 +
>  arch/mips/include/asm/Kbuild       |  1 +
>  arch/nios2/include/asm/Kbuild      |  1 +
>  arch/openrisc/include/asm/Kbuild   |  1 +
>  arch/parisc/include/asm/Kbuild     |  1 +
>  arch/powerpc/include/asm/Kbuild    |  1 +
>  arch/riscv/include/asm/Kbuild      |  1 +
>  arch/s390/include/asm/Kbuild       |  1 +
>  arch/sh/include/asm/Kbuild         |  1 +
>  arch/sparc/include/asm/Kbuild      |  1 +
>  arch/um/include/asm/Kbuild         |  2 +-
>  arch/x86/Kconfig                   | 14 ++++++++++++++
>  arch/xtensa/include/asm/Kbuild     |  1 +
>  include/asm-generic/asi.h          |  5 +++++
>  22 files changed, 41 insertions(+), 1 deletion(-)

I don't think this all is needed. You can put asi.h with stubs used outside
of arch/x86 in include/linux and save you the hassle of updating every
architecture.
 
> diff --git a/arch/sparc/include/asm/Kbuild b/arch/sparc/include/asm/Kbuild
> index 43b0ae4c2c2112d4d4d3cb3c60e787b175172dea..cb9062c9be17fe276cc92d2ac99d8b165f6297bf 100644
> --- a/arch/sparc/include/asm/Kbuild
> +++ b/arch/sparc/include/asm/Kbuild
> @@ -4,3 +4,4 @@ generated-y += syscall_table_64.h
>  generic-y += agp.h
>  generic-y += kvm_para.h
>  generic-y += mcs_spinlock.h
> +generic-y += asi.h

sparc already has include/asm/asi.h, this will break the build

> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 7b9a7e8f39acc8e9aeb7d4213e87d71047865f5c..5a50582eb210e9d1309856a737d32b76fa1bfc85 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2519,6 +2519,20 @@ config MITIGATION_PAGE_TABLE_ISOLATION
>  
>  	  See Documentation/arch/x86/pti.rst for more details.
>  
> +config MITIGATION_ADDRESS_SPACE_ISOLATION
> +	bool "Allow code to run with a reduced kernel address space"
> +	default n
> +	depends on X86_64 && !PARAVIRT && !UML
> +	help
> +	  This feature provides the ability to run some kernel code
> +	  with a reduced kernel address space. This can be used to
> +	  mitigate some speculative execution attacks.
> +
> +	  The !PARAVIRT dependency is only because of lack of testing; in theory
> +	  the code is written to work under paravirtualization. In practice
> +	  there are likely to be unhandled cases, in particular concerning TLB
> +	  flushes.
> +

If you expect other architectures might implement ASI the config would better
fit into init/Kconfig or mm/Kconfig and in arch/x86/Kconfig will define
ARCH_HAS_MITIGATION_ADDRESS_SPACE_ISOLATION.

>  config MITIGATION_RETPOLINE
>  	bool "Avoid speculative indirect branches in kernel"
>  	select OBJTOOL if HAVE_OBJTOOL
> diff --git a/arch/xtensa/include/asm/Kbuild b/arch/xtensa/include/asm/Kbuild
> index fa07c686cbcc2153776a478ac4093846f01eddab..07cea6902f98053be244d026ed594fe7246755a6 100644
> --- a/arch/xtensa/include/asm/Kbuild
> +++ b/arch/xtensa/include/asm/Kbuild
> @@ -8,3 +8,4 @@ generic-y += parport.h
>  generic-y += qrwlock.h
>  generic-y += qspinlock.h
>  generic-y += user.h
> +generic-y += asi.h
> diff --git a/include/asm-generic/asi.h b/include/asm-generic/asi.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..c4d9a5ff860a96428422a15000c622aeecc2d664
> --- /dev/null
> +++ b/include/asm-generic/asi.h
> @@ -0,0 +1,5 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_GENERIC_ASI_H
> +#define __ASM_GENERIC_ASI_H
> +
> +#endif

IMHO it should be include/linux/asi.h, with something like

#infdef __LINUX_ASI_H
#define __LINUX_ASI_H

#ifdef CONFIG_MITIGATION_ADDRESS_SPACE_ISOLATION

#include <asm/asi.h>

#else /* CONFIG_MITIGATION_ADDRESS_SPACE_ISOLATION */

/* stubs for functions used outside arch/ */

#endif /* CONFIG_MITIGATION_ADDRESS_SPACE_ISOLATION */

#endif /* __LINUX_ASI_H */

-- 
Sincerely yours,
Mike.