Re: [PATCH RFC 11/11] xen: use hw_pte_t for PTE range callbacks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Missing VMA boundary checks in privcmd_ioctl_mmap_resource allows arbitrary page table overwrites.
- [High] User-triggerable BUG_ON in alloc_empty_pages via multiple privcmd ioctls on the same VMA.
--

--- Patch [11]: [PATCH RFC 11/11] xen: use hw_pte_t for PTE range callbacks ---
commit f1d3c6242b9edd333d296aa7d48a89b6d4261d91
Author: Muhammad Usama Anjum <usama.anjum@arm.com>

xen: use hw_pte_t for PTE range callbacks

This commit updates generic PTE range and remapping helpers in Xen drivers 
to use hw_pte_t * for PTE table storage pointers, replacing pte_t *. It 
retains logical PTE access via existing helpers.

[ ... ]
> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
> index 725a49a0eee72..b4a487dcc9ecb 100644
> --- a/drivers/xen/privcmd.c
> +++ b/drivers/xen/privcmd.c
> @@ -1658,7 +1658,7 @@ static int privcmd_mmap(struct file *file, struct vm_area_struct *vma)
>   * on a per pfn/pte basis. Mapping calls that fail with ENOENT
>   * can be then retried until success.
>   */
> -static int is_mapped_fn(pte_t *pte, unsigned long addr, void *data)
> +static int is_mapped_fn(hw_pte_t *pte, unsigned long addr, void *data)
>  {
>  	return pte_none(ptep_get(pte)) ? 0 : -EBUSY;
>  }

[Severity: High]
This is a pre-existing issue, but can a user trigger a kernel panic via
multiple ioctls on the same VMA?

Looking at drivers/xen/privcmd.c, privcmd_ioctl_mmap_resource() 
unconditionally calls alloc_empty_pages() which contains a BUG_ON:

privcmd_ioctl_mmap_resource() {
    ...
		rc = alloc_empty_pages(vma, nr);
		if (rc < 0)
			goto out;
    ...
}

alloc_empty_pages() {
    ...
	BUG_ON(vma->vm_private_data != NULL);
	vma->vm_private_data = pages;
    ...
}

If a user maps /dev/xen/privcmd and calls IOCTL_PRIVCMD_MMAP_RESOURCE twice
with the same mapped address, will the second call trigger this BUG_ON and
cause a local denial of service?

[Severity: Critical]
This isn't a bug introduced by this patch, but does
privcmd_ioctl_mmap_resource() miss VMA boundary checks that could allow
arbitrary page table overwrites?

Looking at privcmd_ioctl_mmap_resource() in drivers/xen/privcmd.c:

	vma = find_vma(mm, kdata.addr);
	if (!vma || vma->vm_ops != &privcmd_vm_ops) {
		rc = -EINVAL;
		goto out;
	}

Since find_vma() only verifies that vma->vm_end > kdata.addr, does this
allow a caller to pass a size (num) that extends far beyond the end of the
VMA? 

If so, could the subsequent calls to xen_remap_domain_mfn_array() or 
xen_remap_vma_range() (which use apply_to_page_range()) overwrite PTEs of 
adjacent VMAs with Xen foreign memory mappings?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727164715.2866609-1-usama.anjum@arm.com?part=11