Re: [PATCH 2/2] hisi_acc_vfio_pci: fix VF BAR2 mmap on 64KB page size

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Guest memory-mapping of BAR2 allows direct access to sensitive migration control registers, bypassing software isolation.
--

--- Patch [2]: [PATCH 2/2] hisi_acc_vfio_pci: fix VF BAR2 mmap on 64KB page size ---
commit 6249c89b3b40e6c3a5627d170614fe16d3e815bf
Author: Longfang Liu <[email protected]>

hisi_acc_vfio_pci: fix VF BAR2 mmap on 64KB page size

This commit aligns the BAR2 functional region mmap boundaries to PAGE_SIZE.
This is intended to prevent guest mmap operations from being rejected on 64KB
page size kernels when the functional region is smaller.

> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> index 36490be7a61aa..44b3e7d8fef5c 100644
> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> @@ -1355,14 +1355,21 @@ static int hisi_acc_vfio_pci_mmap(struct vfio_device *core_vdev,
>  	index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT);
>  	if (index == VFIO_PCI_BAR2_REGION_INDEX) {
>  		u64 req_len, pgoff, req_start;
> -		resource_size_t end;
> +		resource_size_t end, dev_len;
>  
> -		end = hisi_acc_get_resource_len(vdev, index);
> +		dev_len = hisi_acc_get_resource_len(vdev, index);
>  		req_len = vma->vm_end - vma->vm_start;
>  		pgoff = vma->vm_pgoff &
>  			((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
>  		req_start = pgoff << PAGE_SHIFT;
> -
> +		/*
> +		 * The BAR2 functional region (dev_len) may be smaller than the
> +		 * kernel page size. Align it to PAGE_SIZE so a page-rounded
> +		 * guest mmap is not rejected, which would make the VF unusable.
> +		 * The read/write path still truncates at the real functional
> +		 * boundary, keeping the migration registers inaccessible.
> +		 */
> +		end = PAGE_ALIGN(dev_len);
> +		if (req_start + req_len > end)
> +			return -EINVAL;

[Severity: Critical]
Does this change allow an untrusted guest or userspace process to bypass
software isolation and gain direct access to migration control registers?

By rounding dev_len up to PAGE_SIZE in hisi_acc_vfio_pci_mmap(), the boundary
check intended to prevent mapping of the hardware migration registers is
circumvented.

If a guest running on a 64KB page host requests an mmap of the BAR2
functional region, the code rounds dev_len up to PAGE_SIZE. This allows the
request to be validated and passed to vfio_pci_core_mmap(), mapping the
full 64KB physical page into the guest's stage-2 page tables.

If the functional region is 32KB and the migration registers reside in the
upper 32KB of the same physical page, the guest would gain unmediated
read/write access to the migration control registers via direct memory
load/store instructions. This appears to bypass the kernel's read/write
interception and the hisi_acc_pci_rw_access_check() boundary checks.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2