diff mbox series

[v2] riscv: mm: Ensure prot of VM_WRITE and VM_EXEC must be readable

Message ID 20230425102828.1616812-1-woodrow.shen@sifive.com (mailing list archive)
State Accepted
Commit 6569fc12e442ea973d96db39e542aa19a7bc3a79
Delegated to: Palmer Dabbelt
Headers show
Series [v2] riscv: mm: Ensure prot of VM_WRITE and VM_EXEC must be readable | expand

Checks

Context Check Description
conchuod/cover_letter success Single patches do not need cover letters
conchuod/tree_selection success Guessed tree name to be fixes at HEAD 1b50f956c8fe
conchuod/fixes_present success Fixes tag present in non-next series
conchuod/maintainers_pattern success MAINTAINERS pattern errors before the patch: 1 and now 1
conchuod/verify_signedoff success Signed-off-by tag matches author and committer
conchuod/kdoc success Errors and warnings before: 0 this patch: 0
conchuod/build_rv64_clang_allmodconfig success Errors and warnings before: 3288 this patch: 3288
conchuod/module_param success Was 0 now: 0
conchuod/build_rv64_gcc_allmodconfig success Errors and warnings before: 17609 this patch: 17609
conchuod/build_rv32_defconfig success Build OK
conchuod/dtb_warn_rv64 success Errors and warnings before: 3 this patch: 3
conchuod/header_inline success No static functions without inline keyword in header files
conchuod/checkpatch success total: 0 errors, 0 warnings, 0 checks, 17 lines checked
conchuod/source_inline success Was 0 now: 0
conchuod/build_rv64_nommu_k210_defconfig success Build OK
conchuod/verify_fixes success Fixes tag looks correct
conchuod/build_rv64_nommu_virt_defconfig success Build OK

Commit Message

Woodrow Shen April 25, 2023, 10:28 a.m. UTC 
From: Hsieh-Tseng Shen <woodrow.shen@sifive.com>

Commit 8aeb7b17f04e ("RISC-V: Make mmap() with PROT_WRITE imply PROT_READ")
allows riscv to use mmap with PROT_WRITE only, and meanwhile mmap with w+x
is also permitted. However, when userspace tries to access this page with
PROT_WRITE|PROT_EXEC, which causes infinite loop at load page fault as
well as it triggers soft lockup. According to riscv privileged spec,
"Writable pages must also be marked readable". The fix to drop the
`PAGE_COPY_READ_EXEC` and then `PAGE_COPY_EXEC` would be just used instead.
This aligns the other arches (i.e arm64) for protection_map.

Fixes: 8aeb7b17f04e ("RISC-V: Make mmap() with PROT_WRITE imply PROT_READ")
Signed-off-by: Hsieh-Tseng Shen <woodrow.shen@sifive.com>
Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
Changelog V2:
 - Rename PAGE_COPY_READ_EXEC into PAGE_COPY_EXEC and remove
   PAGE_COPY_READ_EXEC.
---
 arch/riscv/include/asm/pgtable.h | 3 +--
 arch/riscv/mm/init.c             | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

Comments

Woodrow Shen June 6, 2023, 5:55 a.m. UTC | #1 
Hi all,

Can someone help to review this and see if it can be queued into 6.5? Thanks.

Woodrow


On Tue, Apr 25, 2023 at 6:28 PM Woodrow Shen <woodrow.shen@sifive.com> wrote:
>
> From: Hsieh-Tseng Shen <woodrow.shen@sifive.com>
>
> Commit 8aeb7b17f04e ("RISC-V: Make mmap() with PROT_WRITE imply PROT_READ")
> allows riscv to use mmap with PROT_WRITE only, and meanwhile mmap with w+x
> is also permitted. However, when userspace tries to access this page with
> PROT_WRITE|PROT_EXEC, which causes infinite loop at load page fault as
> well as it triggers soft lockup. According to riscv privileged spec,
> "Writable pages must also be marked readable". The fix to drop the
> `PAGE_COPY_READ_EXEC` and then `PAGE_COPY_EXEC` would be just used instead.
> This aligns the other arches (i.e arm64) for protection_map.
>
> Fixes: 8aeb7b17f04e ("RISC-V: Make mmap() with PROT_WRITE imply PROT_READ")
> Signed-off-by: Hsieh-Tseng Shen <woodrow.shen@sifive.com>
> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
> Changelog V2:
>  - Rename PAGE_COPY_READ_EXEC into PAGE_COPY_EXEC and remove
>    PAGE_COPY_READ_EXEC.
> ---
>  arch/riscv/include/asm/pgtable.h | 3 +--
>  arch/riscv/mm/init.c             | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index f641837ccf31..05eda3281ba9 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -165,8 +165,7 @@ extern struct pt_alloc_ops pt_ops __initdata;
>                                          _PAGE_EXEC | _PAGE_WRITE)
>
>  #define PAGE_COPY              PAGE_READ
> -#define PAGE_COPY_EXEC         PAGE_EXEC
> -#define PAGE_COPY_READ_EXEC    PAGE_READ_EXEC
> +#define PAGE_COPY_EXEC         PAGE_READ_EXEC
>  #define PAGE_SHARED            PAGE_WRITE
>  #define PAGE_SHARED_EXEC       PAGE_WRITE_EXEC
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 0f14f4a8d179..cc48b0d93a98 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -286,7 +286,7 @@ static const pgprot_t protection_map[16] = {
>         [VM_EXEC]                                       = PAGE_EXEC,
>         [VM_EXEC | VM_READ]                             = PAGE_READ_EXEC,
>         [VM_EXEC | VM_WRITE]                            = PAGE_COPY_EXEC,
> -       [VM_EXEC | VM_WRITE | VM_READ]                  = PAGE_COPY_READ_EXEC,
> +       [VM_EXEC | VM_WRITE | VM_READ]                  = PAGE_COPY_EXEC,
>         [VM_SHARED]                                     = PAGE_NONE,
>         [VM_SHARED | VM_READ]                           = PAGE_READ,
>         [VM_SHARED | VM_WRITE]                          = PAGE_SHARED,
> --
> 2.34.1
>
Palmer Dabbelt June 8, 2023, 2:04 p.m. UTC | #2 
On Tue, 25 Apr 2023 18:28:28 +0800, Woodrow Shen wrote:
> Commit 8aeb7b17f04e ("RISC-V: Make mmap() with PROT_WRITE imply PROT_READ")
> allows riscv to use mmap with PROT_WRITE only, and meanwhile mmap with w+x
> is also permitted. However, when userspace tries to access this page with
> PROT_WRITE|PROT_EXEC, which causes infinite loop at load page fault as
> well as it triggers soft lockup. According to riscv privileged spec,
> "Writable pages must also be marked readable". The fix to drop the
> `PAGE_COPY_READ_EXEC` and then `PAGE_COPY_EXEC` would be just used instead.
> This aligns the other arches (i.e arm64) for protection_map.
> 
> [...]

Applied, thanks!

[1/1] riscv: mm: Ensure prot of VM_WRITE and VM_EXEC must be readable
      https://git.kernel.org/palmer/c/6569fc12e442

Best regards,
patchwork-bot+linux-riscv@kernel.org June 8, 2023, 2:10 p.m. UTC | #3 
Hello:

This patch was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Tue, 25 Apr 2023 18:28:28 +0800 you wrote:
> From: Hsieh-Tseng Shen <woodrow.shen@sifive.com>
> 
> Commit 8aeb7b17f04e ("RISC-V: Make mmap() with PROT_WRITE imply PROT_READ")
> allows riscv to use mmap with PROT_WRITE only, and meanwhile mmap with w+x
> is also permitted. However, when userspace tries to access this page with
> PROT_WRITE|PROT_EXEC, which causes infinite loop at load page fault as
> well as it triggers soft lockup. According to riscv privileged spec,
> "Writable pages must also be marked readable". The fix to drop the
> `PAGE_COPY_READ_EXEC` and then `PAGE_COPY_EXEC` would be just used instead.
> This aligns the other arches (i.e arm64) for protection_map.
> 
> [...]

Here is the summary with links:
  - [v2] riscv: mm: Ensure prot of VM_WRITE and VM_EXEC must be readable
    https://git.kernel.org/riscv/c/6569fc12e442

You are awesome, thank you!
diff mbox series

Patch

diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index f641837ccf31..05eda3281ba9 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -165,8 +165,7 @@  extern struct pt_alloc_ops pt_ops __initdata;
 					 _PAGE_EXEC | _PAGE_WRITE)
 
 #define PAGE_COPY		PAGE_READ
-#define PAGE_COPY_EXEC		PAGE_EXEC
-#define PAGE_COPY_READ_EXEC	PAGE_READ_EXEC
+#define PAGE_COPY_EXEC		PAGE_READ_EXEC
 #define PAGE_SHARED		PAGE_WRITE
 #define PAGE_SHARED_EXEC	PAGE_WRITE_EXEC
 
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 0f14f4a8d179..cc48b0d93a98 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -286,7 +286,7 @@  static const pgprot_t protection_map[16] = {
 	[VM_EXEC]					= PAGE_EXEC,
 	[VM_EXEC | VM_READ]				= PAGE_READ_EXEC,
 	[VM_EXEC | VM_WRITE]				= PAGE_COPY_EXEC,
-	[VM_EXEC | VM_WRITE | VM_READ]			= PAGE_COPY_READ_EXEC,
+	[VM_EXEC | VM_WRITE | VM_READ]			= PAGE_COPY_EXEC,
 	[VM_SHARED]					= PAGE_NONE,
 	[VM_SHARED | VM_READ]				= PAGE_READ,
 	[VM_SHARED | VM_WRITE]				= PAGE_SHARED,