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 <[email protected]> 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/[email protected]?part=11
