Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v2 08/12] powerpc/32s: Allow disabling KUAP at boot time
From:   Christophe Leroy <christophe.leroy@csgroup.eu>
To:     Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Paul Mackerras <paulus@samba.org>,
        Michael Ellerman <mpe@ellerman.id.au>
Cc:     linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
References: <cover.1622708530.git.christophe.leroy@csgroup.eu>
 <cd79e8008455fba5395d099f9bb1305c039b931c.1622708530.git.christophe.leroy@csgroup.eu>
Message-ID: <ee1fb979-3503-2c56-cde5-1ea7eed790d7@csgroup.eu>
Date:   Sat, 12 Jun 2021 08:18:44 +0200
User-Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <cd79e8008455fba5395d099f9bb1305c039b931c.1622708530.git.christophe.leroy@csgroup.eu>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: fr
Content-Transfer-Encoding: 8bit
Precedence: bulk


Le 03/06/2021 à 10:41, Christophe Leroy a écrit :
> PPC64 uses MMU features to enable/disable KUAP at boot time.
> But feature fixups are applied way too early on PPC32.
> 
> Now that all KUAP related actions are in C following the
> conversion of KUAP initial setup and context switch in C,
> static branches can be used to enable/disable KUAP.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>   arch/powerpc/include/asm/book3s/32/kup.h | 27 +++++++++++++++++++++++-
>   arch/powerpc/mm/book3s32/kuap.c          | 11 ++++++----
>   2 files changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/book3s/32/kup.h b/arch/powerpc/include/asm/book3s/32/kup.h
> index 2854d970dabe..68fbe28c6d7e 100644
> --- a/arch/powerpc/include/asm/book3s/32/kup.h
> +++ b/arch/powerpc/include/asm/book3s/32/kup.h
> @@ -9,11 +9,12 @@
>   
>   #include <linux/jump_label.h>
>   
> +extern struct static_key_false disable_kuap_key;

Same as 8xx, this needs to be exported for modules.

>   extern struct static_key_false disable_kuep_key;
>   
>   static __always_inline bool kuap_is_disabled(void)
>   {
> -	return !IS_ENABLED(CONFIG_PPC_KUAP);
> +	return !IS_ENABLED(CONFIG_PPC_KUAP) || static_branch_unlikely(&disable_kuap_key);
>   }
>   
>   static __always_inline bool kuep_is_disabled(void)
> @@ -62,6 +63,9 @@ static inline void kuap_save_and_lock(struct pt_regs *regs)
>   	u32 addr = kuap & 0xf0000000;
>   	u32 end = kuap << 28;
>   
> +	if (kuap_is_disabled())
> +		return;
> +
>   	regs->kuap = kuap;
>   	if (unlikely(!kuap))
>   		return;
> @@ -79,6 +83,9 @@ static inline void kuap_kernel_restore(struct pt_regs *regs, unsigned long kuap)
>   	u32 addr = regs->kuap & 0xf0000000;
>   	u32 end = regs->kuap << 28;
>   
> +	if (kuap_is_disabled())
> +		return;
> +
>   	current->thread.kuap = regs->kuap;
>   
>   	if (unlikely(regs->kuap == kuap))
> @@ -91,6 +98,9 @@ static inline unsigned long kuap_get_and_assert_locked(void)
>   {
>   	unsigned long kuap = current->thread.kuap;
>   
> +	if (kuap_is_disabled())
> +		return 0;
> +
>   	WARN_ON_ONCE(IS_ENABLED(CONFIG_PPC_KUAP_DEBUG) && kuap != 0);
>   
>   	return kuap;
> @@ -106,6 +116,9 @@ static __always_inline void allow_user_access(void __user *to, const void __user
>   {
>   	u32 addr, end;
>   
> +	if (kuap_is_disabled())
> +		return;
> +
>   	BUILD_BUG_ON(!__builtin_constant_p(dir));
>   	BUILD_BUG_ON(dir & ~KUAP_READ_WRITE);
>   
> @@ -128,6 +141,9 @@ static __always_inline void prevent_user_access(void __user *to, const void __us
>   {
>   	u32 addr, end;
>   
> +	if (kuap_is_disabled())
> +		return;
> +
>   	BUILD_BUG_ON(!__builtin_constant_p(dir));
>   
>   	if (dir & KUAP_CURRENT_WRITE) {
> @@ -159,6 +175,9 @@ static inline unsigned long prevent_user_access_return(void)
>   	unsigned long end = flags << 28;
>   	void __user *to = (__force void __user *)addr;
>   
> +	if (kuap_is_disabled())
> +		return 0;
> +
>   	if (flags)
>   		prevent_user_access(to, to, end - addr, KUAP_READ_WRITE);
>   
> @@ -171,6 +190,9 @@ static inline void restore_user_access(unsigned long flags)
>   	unsigned long end = flags << 28;
>   	void __user *to = (__force void __user *)addr;
>   
> +	if (kuap_is_disabled())
> +		return;
> +
>   	if (flags)
>   		allow_user_access(to, to, end - addr, KUAP_READ_WRITE);
>   }
> @@ -181,6 +203,9 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
>   	unsigned long begin = regs->kuap & 0xf0000000;
>   	unsigned long end = regs->kuap << 28;
>   
> +	if (kuap_is_disabled())
> +		return false;
> +
>   	return is_write && (address < begin || address >= end);
>   }
>   
> diff --git a/arch/powerpc/mm/book3s32/kuap.c b/arch/powerpc/mm/book3s32/kuap.c
> index 5533ed92ab3d..a4ce6cdc28e5 100644
> --- a/arch/powerpc/mm/book3s32/kuap.c
> +++ b/arch/powerpc/mm/book3s32/kuap.c
> @@ -3,15 +3,18 @@
>   #include <asm/kup.h>
>   #include <asm/smp.h>
>   
> +struct static_key_false disable_kuap_key;
> +
>   void __init setup_kuap(bool disabled)
>   {
> -	kuap_update_sr(mfsr(0) | SR_KS, 0, TASK_SIZE);
> +	if (!disabled)
> +		kuap_update_sr(mfsr(0) | SR_KS, 0, TASK_SIZE);
>   
>   	if (smp_processor_id() != boot_cpuid)
>   		return;
>   
> -	pr_info("Activating Kernel Userspace Access Protection\n");
> -
>   	if (disabled)
> -		pr_warn("KUAP cannot be disabled yet on 6xx when compiled in\n");
> +		static_branch_enable(&disable_kuap_key);
> +	else
> +		pr_info("Activating Kernel Userspace Access Protection\n");
>   }
>