Re: [PATCH] RDMA/rxe: Restore HMM_PFN_WRITE check in ODP write paths

While exploiting this ODP flaw in soft-RoCE for privilege escalation
in practice might require specific conditions, enforcing the HMM_PFN_WRITE
check is undoubtedly the right choice to enforce strict access control
and eliminate the security risk.

The special handling for pmem flush (keeping it read-only) is also a nice
and correct touch.

Let us wait for the feedback from Leon and Jason.

Reviewed-by: Zhu Yanjun <[email protected]>

Zhu Yanjun

在 2026/7/25 17:08, Zhu Yanjun 写道:
>
> 在 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