[RFC PATCH v1] riscv: mm: Ensure prot of VM_WRITE and VM_EXEC must be readable

[Woodrow Shen]

Hi Conor,

On Tue, Apr 25, 2023 at 2:55 PM Conor Dooley <conor.dooley at microchip.com> wrote:
>
> Hey Hsieh-Tseng,
>
> On Tue, Apr 25, 2023 at 11:44:07AM +0800, Woodrow Shen wrote:
> > From: Hsieh-Tseng Shen <woodrow.shen at sifive.com>
> >
> > The commit 8aeb7b1 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 `PAGE_READ_EXEC`
> > should be just used instead. This aligns the other arches (i.e arm64)
> > for protection_map.
> >
> > Fixes: 8aeb7b1 ("RISC-V: Make mmap() with PROT_WRITE imply PROT_READ")
>
> This fixes tag should be:
> Fixes: 8aeb7b17f04e ("RISC-V: Make mmap() with PROT_WRITE imply PROT_READ")
> as checkpatch would have told you :/ Perhaps Palmer can fix that up on
> application?
>
> > Signed-off-by: Hsieh-Tseng Shen <woodrow.shen at sifive.com>
> > Reviewed-by: Alexandre Ghiti <alexghiti at rivosinc.com>
> > ---
>
> This is not v1, it's v2 right?
> Please include a changelog between versions.

Yes, it's v2. I copied the outdated comment as my first version was
verified by script correctly. Will fix it in v2.

>
> Also, what makes it an RFC, it's an actual fix that you would like
> merged, right?

I was suggested to use "RFC" internally, so should I just re-submit v2
with [RESEND v2]?
Thanks,
Woodrow

>
> Thanks,
> Conor.
>
> >  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
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv