Commit 2139619bcad7 ("riscv: mmap with PROT_WRITE but no PROT_READ is
invalid") made mmap() reject mappings with only PROT_WRITE set in an
attempt to fix an observed inconsistency in behavior when attempting
to read from a PROT_WRITE-only mapping. The root cause of this behavior
was actually that while RISC-V's protection_map maps VM_WRITE to
readable PTE permissions (since write-only PTEs are considered reserved
by the privileged spec), the page fault handler considered loads from
VM_WRITE-only VMAs illegal accesses. Fix the underlying cause by
handling faults in VM_WRITE-only VMAs (patch 1) and then re-enable
use of mmap(PROT_WRITE) (patch 2), making RISC-V's behavior consistent
with all other architectures that don't support write-only PTEs.
Both patches are tagged as fixes for the aforementioned commit since that
commit made a userspace visible change that will break any software relying
on mmap(PROT_WRITE). (Also cc: stable since the offending commit was
itself backported to stable).
v1 -> v2: Allow handling of load faults in VM_WRITE VMAs
v2 -> v3: Split into two pathces
v3 -> v4: Fixes tags (+ this cover letter)
Andrew Bresticker (2):
riscv: Make VM_WRITE imply VM_READ
riscv: Allow PROT_WRITE-only mmap()
arch/riscv/kernel/sys_riscv.c | 3 ---
arch/riscv/mm/fault.c | 3 ++-
2 files changed, 2 insertions(+), 4 deletions(-)
--
2.25.1
RISC-V does not presently have write-only mappings as that PTE bit pattern
is considered reserved in the privileged spec, so allow handling of read
faults in VMAs that have VM_WRITE without VM_READ in order to be consistent
with other architectures that have similar limitations.
Fixes: 2139619bcad7 ("riscv: mmap with PROT_WRITE but no PROT_READ is invalid")
Cc: <[email protected]> # v4.19+
Reviewed-by: Atish Patra <[email protected]>
Signed-off-by: Andrew Bresticker <[email protected]>
---
new in v3
v3 -> v4: add Fixes tag
---
arch/riscv/mm/fault.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index f2fbd1400b7c..d86f7cebd4a7 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -184,7 +184,8 @@ static inline bool access_error(unsigned long cause, struct vm_area_struct *vma)
}
break;
case EXC_LOAD_PAGE_FAULT:
- if (!(vma->vm_flags & VM_READ)) {
+ /* Write implies read */
+ if (!(vma->vm_flags & (VM_READ | VM_WRITE))) {
return true;
}
break;
--
2.25.1
Hey Palmer,
On Thu, Sep 15, 2022 at 03:37:00PM -0400, Andrew Bresticker wrote:
> Commit 2139619bcad7 ("riscv: mmap with PROT_WRITE but no PROT_READ is
> invalid") made mmap() reject mappings with only PROT_WRITE set in an
> attempt to fix an observed inconsistency in behavior when attempting
> to read from a PROT_WRITE-only mapping. The root cause of this behavior
> was actually that while RISC-V's protection_map maps VM_WRITE to
> readable PTE permissions (since write-only PTEs are considered reserved
> by the privileged spec), the page fault handler considered loads from
> VM_WRITE-only VMAs illegal accesses. Fix the underlying cause by
> handling faults in VM_WRITE-only VMAs (patch 1) and then re-enable
> use of mmap(PROT_WRITE) (patch 2), making RISC-V's behavior consistent
> with all other architectures that don't support write-only PTEs.
>
> Both patches are tagged as fixes for the aforementioned commit since that
> commit made a userspace visible change that will break any software relying
> on mmap(PROT_WRITE). (Also cc: stable since the offending commit was
> itself backported to stable).
The patch that these commits fix has hit the distros & manifests as a
userspace breakage for openJDK:
https://lore.kernel.org/linux-riscv/[email protected]/
https://lore.kernel.org/linux-riscv/[email protected]/
Eva tested these patches and reported that their problem was fixed:
https://lore.kernel.org/linux-riscv/[email protected]/
I asked them for a T-b but I don't see one on lore etc, but it would be
from Eva Kotova <[email protected]> if you consider their
comments their sufficient for a T-B
Thanks,
Conor.
>
> v1 -> v2: Allow handling of load faults in VM_WRITE VMAs
> v2 -> v3: Split into two pathces
> v3 -> v4: Fixes tags (+ this cover letter)
>
> Andrew Bresticker (2):
> riscv: Make VM_WRITE imply VM_READ
> riscv: Allow PROT_WRITE-only mmap()
>
> arch/riscv/kernel/sys_riscv.c | 3 ---
> arch/riscv/mm/fault.c | 3 ++-
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> --
> 2.25.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Thu, 15 Sep 2022 12:37:00 PDT (-0700), [email protected] wrote:
> Commit 2139619bcad7 ("riscv: mmap with PROT_WRITE but no PROT_READ is
> invalid") made mmap() reject mappings with only PROT_WRITE set in an
> attempt to fix an observed inconsistency in behavior when attempting
> to read from a PROT_WRITE-only mapping. The root cause of this behavior
> was actually that while RISC-V's protection_map maps VM_WRITE to
> readable PTE permissions (since write-only PTEs are considered reserved
> by the privileged spec), the page fault handler considered loads from
> VM_WRITE-only VMAs illegal accesses. Fix the underlying cause by
> handling faults in VM_WRITE-only VMAs (patch 1) and then re-enable
> use of mmap(PROT_WRITE) (patch 2), making RISC-V's behavior consistent
> with all other architectures that don't support write-only PTEs.
>
> Both patches are tagged as fixes for the aforementioned commit since that
> commit made a userspace visible change that will break any software relying
> on mmap(PROT_WRITE). (Also cc: stable since the offending commit was
> itself backported to stable).
>
> v1 -> v2: Allow handling of load faults in VM_WRITE VMAs
> v2 -> v3: Split into two pathces
> v3 -> v4: Fixes tags (+ this cover letter)
>
> Andrew Bresticker (2):
> riscv: Make VM_WRITE imply VM_READ
> riscv: Allow PROT_WRITE-only mmap()
>
> arch/riscv/kernel/sys_riscv.c | 3 ---
> arch/riscv/mm/fault.c | 3 ++-
> 2 files changed, 2 insertions(+), 4 deletions(-)
Thanks, these are on for-next.