linux-kernel - Re: [PATCH] riscv: Define TASK_SIZE_MAX for __access

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHVXubjLWZkjSapnsWJzimWg_2swEy7tQ-DQ6ri8yMk8-Qsc-A@mail.gmail.com>
Date: Mon, 18 Mar 2024 21:50:06 +0100
From: Alexandre Ghiti <alexghiti@...osinc.com>
To: Samuel Holland <samuel.holland@...ive.com>
Cc: Palmer Dabbelt <palmer@...belt.com>, linux-riscv@...ts.infradead.org, 
	Albert Ou <aou@...s.berkeley.edu>, Andrew Morton <akpm@...ux-foundation.org>, 
	Charlie Jenkins <charlie@...osinc.com>, Guo Ren <guoren@...nel.org>, 
	Jisheng Zhang <jszhang@...nel.org>, Kemeng Shi <shikemeng@...weicloud.com>, 
	"Matthew Wilcox (Oracle)" <willy@...radead.org>, "Mike Rapoport (IBM)" <rppt@...nel.org>, 
	Paul Walmsley <paul.walmsley@...ive.com>, Xiao Wang <xiao.w.wang@...el.com>, 
	Yangyu Chen <cyy@...self.name>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] riscv: Define TASK_SIZE_MAX for __access_ok()

Hi Samuel,

On Wed, Mar 13, 2024 at 7:00 PM Samuel Holland
<samuel.holland@...ive.com> wrote:
>
> TASK_SIZE_MAX should be set to the largest userspace address under any
> runtime configuration. This optimizes the check in __access_ok(), which
> no longer needs to compute the current value of TASK_SIZE. It is still
> safe because addresses between TASK_SIZE and TASK_SIZE_MAX are invalid
> at the hardware level.
>
> This removes about half of the references to pgtable_l[45]_enabled.
>
> Signed-off-by: Samuel Holland <samuel.holland@...ive.com>
> ---
>
>  arch/riscv/include/asm/pgtable-64.h | 1 +
>  arch/riscv/include/asm/pgtable.h    | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
> index b99bd66107a6..a677ef3c0fe2 100644
> --- a/arch/riscv/include/asm/pgtable-64.h
> +++ b/arch/riscv/include/asm/pgtable-64.h
> @@ -17,6 +17,7 @@ extern bool pgtable_l5_enabled;
>  #define PGDIR_SHIFT_L4  39
>  #define PGDIR_SHIFT_L5  48
>  #define PGDIR_SIZE_L3   (_AC(1, UL) << PGDIR_SHIFT_L3)
> +#define PGDIR_SIZE_L5   (_AC(1, UL) << PGDIR_SHIFT_L5)
>
>  #define PGDIR_SHIFT     (pgtable_l5_enabled ? PGDIR_SHIFT_L5 : \
>                 (pgtable_l4_enabled ? PGDIR_SHIFT_L4 : PGDIR_SHIFT_L3))
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 6066822e7396..2032f8ac5fc5 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -867,6 +867,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
>  #ifdef CONFIG_64BIT
>  #define TASK_SIZE_64   (PGDIR_SIZE * PTRS_PER_PGD / 2)
>  #define TASK_SIZE_MIN  (PGDIR_SIZE_L3 * PTRS_PER_PGD / 2)
> +#define TASK_SIZE_MAX  (PGDIR_SIZE_L5 * PTRS_PER_PGD / 2)
>
>  #ifdef CONFIG_COMPAT
>  #define TASK_SIZE_32   (_AC(0x80000000, UL))
> --
> 2.43.1
>

I think you also need to change the check in handle_page_fault() by
using TASK_SIZE_MAX instead of TASK_SIZE, otherwise the fixup can't
happen (https://elixir.bootlin.com/linux/latest/source/arch/riscv/mm/fault.c#L273).

Or I was wondering if it would not be better to do like x86 and use an
alternative, it would be more correct (even though I believe your
solution works)
https://elixir.bootlin.com/linux/latest/source/arch/x86/include/asm/page_64h#L82.

Thanks,

Alex