Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date:   Tue, 06 Jun 2023 19:27:03 +1000
Message-Id: <CT5GKZN6D21R.3US3D2GZWGLS2@wheely>
Subject: Re: [PATCH 4/4] powerpc/kuap: Make disabling KUAP at boottime
 optional
From:   "Nicholas Piggin" <npiggin@gmail.com>
To:     "Christophe Leroy" <christophe.leroy@csgroup.eu>,
        "Michael Ellerman" <mpe@ellerman.id.au>
Cc:     <linux-kernel@vger.kernel.org>, <linuxppc-dev@lists.ozlabs.org>
References: <1a3c69e38349b687c6c65240d7c09a7817a797d8.1685963081.git.christophe.leroy@csgroup.eu> <8d5438da7174ecb32e1c28cdb49987648df6ef15.1685963081.git.christophe.leroy@csgroup.eu>
In-Reply-To: <8d5438da7174ecb32e1c28cdb49987648df6ef15.1685963081.git.christophe.leroy@csgroup.eu>
Precedence: bulk

On Mon Jun 5, 2023 at 9:04 PM AEST, Christophe Leroy wrote:
> It is possible to disable KUAP at boottime with 'nosmap' parameter.
>
> That is implemented with jump_label hence adds a 'nop' in front
> of each open/close of userspace access.
>
> From a security point of view it makes sence to disallow disabling
> KUAP. And on processors like the 8xx where 'nop' is not seamless,
> it saves a few cycles.
>
> So add a CONFIG item to make it optionnal.

I love counting cycles, but a CONFIG option for one nop... I think
you have me beat.

Is that sustainable? What if instead of the static branch you patched in
nops to the lock/unlock asm? AFAIKS there does not look like much (any?)
C code in the kuap enabled branches.

Thanks,
Nick

>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>  arch/powerpc/include/asm/kup.h         |  2 +-
>  arch/powerpc/mm/init-common.c          |  3 +++
>  arch/powerpc/platforms/Kconfig.cputype | 10 ++++++++++
>  3 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/ku=
p.h
> index 74b7f4cee2ed..f3280169aeec 100644
> --- a/arch/powerpc/include/asm/kup.h
> +++ b/arch/powerpc/include/asm/kup.h
> @@ -53,7 +53,7 @@ extern struct static_key_false disable_kuap_key;
> =20
>  static __always_inline bool kuap_is_disabled(void)
>  {
> -	return static_branch_unlikely(&disable_kuap_key);
> +	return IS_ENABLED(CONFIG_PPC_KUAP_BOOTTIME) && static_branch_unlikely(&=
disable_kuap_key);
>  }
>  #endif
>  #else
> diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.=
c
> index 74e140b1efef..994ee58f0092 100644
> --- a/arch/powerpc/mm/init-common.c
> +++ b/arch/powerpc/mm/init-common.c
> @@ -48,6 +48,9 @@ early_param("nosmep", parse_nosmep);
> =20
>  static int __init parse_nosmap(char *p)
>  {
> +	if (!IS_ENABLED(CONFIG_PPC_KUAP_BOOTTIME))
> +		return 0;
> +
>  	disable_kuap =3D true;
>  	pr_warn("Disabling Kernel Userspace Access Protection\n");
>  	return 0;
> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platfo=
rms/Kconfig.cputype
> index 45fd975ef521..f75c2d5cd182 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -502,6 +502,16 @@ config PPC_KUAP
> =20
>  	  If you're unsure, say Y.
> =20
> +config PPC_KUAP_BOOTTIME
> +	bool "Allow disabling Kernel Userspace Access Protection at boottime"
> +	depends on PPC_KUAP
> +	default y
> +	help
> +	  Allow the user to disable Kernel Userspace Access Protection (KUAP)
> +	  at boot time using 'nosmap' kernel parameter.
> +
> +	  If you're unsure, say Y.
> +
>  config PPC_KUAP_DEBUG
>  	bool "Extra debugging for Kernel Userspace Access Protection"
>  	depends on PPC_KUAP
> --=20
> 2.40.1