Re: [PATCH v2 14/15] riscv/mm: optimize mprotect for Svnapot mappings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The fallback path corrupts userspace memory tracking by erroneously injecting `_PAGE_SOFT_DIRTY` via `pte_mkdirty()`.
--

--- Patch [14]: [PATCH v2 14/15] riscv/mm: optimize mprotect for Svnapot mappings ---
commit f75eddb541d9ba388068a3cbeef6571534eb416b
Author: Yunhui Cui <cuiyunhui@bytedance.com>

riscv/mm: optimize mprotect for Svnapot mappings

This commit optimizes mprotect operations for Svnapot mappings by handling
full blocks in a single batched operation. A fallback loop is retained for
partial, unaligned, or otherwise inconsistent mappings.

> diff --git a/arch/riscv/mm/contpte.c b/arch/riscv/mm/contpte.c
> index 8289f39d6e188..6a1156629d115 100644
> --- a/arch/riscv/mm/contpte.c
> +++ b/arch/riscv/mm/contpte.c

[ ... ]

> +pte_t modify_prot_start_ptes(struct vm_area_struct *vma, unsigned long addr,
> +			     pte_t *ptep, unsigned int nr)
> +{
> +	struct mm_struct *mm = vma->vm_mm;
> +	unsigned long ptent_addr;
> +	pte_t raw, pte, ptent;
> +	unsigned int i;
> +
> +	if (napotpte_can_modify_prot_block(mm, addr, ptep, nr, &raw)) {

[ ... ]

> +			if (pte_dirty(ptent))
> +				pte = riscv_pte_mkhwdirty(pte);

[ ... ]

> +		return pte;
> +	}
> +
> +	pte = __ptep_modify_prot_start(vma, addr, ptep);
> +	while (--nr) {
> +		ptep++;
> +		addr += PAGE_SIZE;
> +		ptent = __ptep_modify_prot_start(vma, addr, ptep);
> +		if (pte_dirty(ptent))
> +			pte = pte_mkdirty(pte);

[Severity: Medium]
Does this fallback path unintentionally set the soft-dirty bit for userspace
tracking?

On RISC-V, pte_mkdirty() sets both the hardware _PAGE_DIRTY bit and the
userspace-visible _PAGE_SOFT_DIRTY bit. If a Svnapot mapping was previously
written to (hardware dirty) but its soft-dirty bit was explicitly cleared by
userspace (e.g. via /proc/pid/clear_refs), calling mprotect() on a partial
or unaligned segment of this block triggers this fallback path.

Since the fallback path uses pte_mkdirty() to accumulate the dirty state, it
would inadvertently re-set the _PAGE_SOFT_DIRTY bit on the final batched PTE.
Userspace tracking tools like CRIU could subsequently falsely record this
memory as modified.

The fast path correctly uses riscv_pte_mkhwdirty(pte) to avoid this exact
issue. Should this loop also use riscv_pte_mkhwdirty(pte) instead of
pte_mkdirty(pte)?

> +		if (pte_young(ptent))
> +			pte = pte_mkyoung(pte);
> +	}
> +
> +	return pte;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784201104.git.cuiyunhui@bytedance.com?part=14