Re: [PATCH] RDMA/rxe: Restore HMM_PFN_WRITE check in ODP write paths
Zhu Yanjun <[email protected]>
| Newsgroups | org.kernel.vger.linux-rdma |
|---|---|
| Message-ID | <[email protected]> |
在 2026/7/25 2:48, Weiming Shi 写道:
> Commit 0b261d7c1cd3 ("RDMA/rxe: Break endless pagefault loop for RO
> pages") dropped the access permission test from rxe_check_pagefault()
> and left only HMM_PFN_VALID. A page faulted in read-only, for example
> a page-cache folio behind a PROT_READ file mapping, then satisfies the
> check and ODP write operations (RDMA WRITE, RDMA READ response, SEND
> payload, atomics) modify it through kmap without ever breaking CoW.
>
> An unprivileged user can register an ODP MR over such a mapping and
> have incoming RDMA traffic overwrite the page cache of a file it only
> holds O_RDONLY, including /etc/passwd or setuid binaries. This is the
> same primitive class as Dirty COW and CVE-2022-2590.
>
> mlx5 has the missing invariant: its ODP path sets the device write bit
> only for pfns that carry HMM_PFN_WRITE. Restore it in rxe by requiring
> HMM_PFN_WRITE in rxe_check_pagefault() for every operation except
> RXE_PAGEFAULT_RDONLY. A write to a non-writable VMA now fails the one
> fault attempt with -EPERM from hmm_vma_fault() instead of re-faulting
> forever. For a writable VMA the fault breaks CoW and the write lands
> in the private page.
>
> Keep pmem flushes on the read-only check. arch_wb_cache_pmem() never
> modifies memory, and the FLUSH access bits do not make the umem
> writable, so classifying flushes as writes would make every flush
> against a flush-only MR fail.
>
> Fixes: 0b261d7c1cd3 ("RDMA/rxe: Break endless pagefault loop for RO pages")
> Cc: [email protected]
> Signed-off-by: Weiming Shi <[email protected]>
> Reported-by: Shaomin Chen <[email protected]>
> Reported-by: Rui Ding <[email protected]>
> Reported-by: Miao Zhao <[email protected]>
> Tested-by: Qiang Hong <[email protected]>
> Tested-by: Xinyu Ma <[email protected]>
> ---
> drivers/infiniband/sw/rxe/rxe_odp.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_odp.c b/drivers/infiniband/sw/rxe/rxe_odp.c
> index ff904d5e54a73..174bb2efc3c6a 100644
> --- a/drivers/infiniband/sw/rxe/rxe_odp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_odp.c
> @@ -124,19 +124,23 @@ int rxe_odp_mr_init_user(struct rxe_dev *rxe, u64 start, u64 length,
> }
>
> static inline bool rxe_check_pagefault(struct ib_umem_odp *umem_odp, u64 iova,
> - int length)
> + int length, bool write)
> {
> bool need_fault = false;
> + u64 access = HMM_PFN_VALID;
> u64 addr;
> int idx;
>
> + if (write)
> + access |= HMM_PFN_WRITE;
> +
> addr = iova & (~(BIT(umem_odp->page_shift) - 1));
>
> /* Skim through all pages that are to be accessed. */
> while (addr < iova + length) {
> idx = (addr - ib_umem_start(umem_odp)) >> umem_odp->page_shift;
>
> - if (!(umem_odp->map.pfn_list[idx] & HMM_PFN_VALID)) {
> + if ((umem_odp->map.pfn_list[idx] & access) != access) {
> need_fault = true;
> break;
> }
> @@ -159,6 +163,7 @@ static unsigned long rxe_odp_iova_to_page_offset(struct ib_umem_odp *umem_odp, u
> static int rxe_odp_map_range_and_lock(struct rxe_mr *mr, u64 iova, int length, u32 flags)
> {
> struct ib_umem_odp *umem_odp = to_ib_umem_odp(mr->umem);
> + bool write = !(flags & RXE_PAGEFAULT_RDONLY);
> bool need_fault;
> int err;
>
> @@ -167,7 +172,7 @@ static int rxe_odp_map_range_and_lock(struct rxe_mr *mr, u64 iova, int length, u
>
> mutex_lock(&umem_odp->umem_mutex);
>
> - need_fault = rxe_check_pagefault(umem_odp, iova, length);
> + need_fault = rxe_check_pagefault(umem_odp, iova, length, write);
> if (need_fault) {
> mutex_unlock(&umem_odp->umem_mutex);
>
> @@ -177,7 +182,7 @@ static int rxe_odp_map_range_and_lock(struct rxe_mr *mr, u64 iova, int length, u
> if (err < 0)
> return err;
>
> - need_fault = rxe_check_pagefault(umem_odp, iova, length);
> + need_fault = rxe_check_pagefault(umem_odp, iova, length, write);
> if (need_fault) {
> mutex_unlock(&umem_odp->umem_mutex);
> return -EFAULT;
> @@ -339,8 +344,9 @@ int rxe_odp_flush_pmem_iova(struct rxe_mr *mr, u64 iova,
> int err;
> u8 *va;
>
> + /* A flush never modifies memory; read-only access suffices. */
> err = rxe_odp_map_range_and_lock(mr, iova, length,
> - RXE_PAGEFAULT_DEFAULT);
> + RXE_PAGEFAULT_RDONLY);
> if (err)
> return err;
>
--
Best Regards,
Yanjun.Zhu