Re: [PATCH v6 3/3] iommufd: Add IOMMU_IOAS_RESERVE_IOVAS ioctl

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] iommufd: off-by-one ULONG_MAX check in iommufd_ioas_reserve_iovas()
--

commit 0ed118205dea2d86a2a0d60398112fcc9b0dd85e
Author: Guanghui Feng <guanghuifeng@linux.alibaba.com>

iommufd: Add IOMMU_IOAS_RESERVE_IOVAS ioctl

This commit adds the IOMMU_IOAS_RESERVE_IOVAS ioctl to allow userspace to
reserve IOVA ranges in an IOAS so that they are excluded from
IOMMU_IOAS_IOVA_RANGES and rejected by mapping operations.

> diff --git a/drivers/iommu/iommufd/ioas.c b/drivers/iommu/iommufd/ioas.c
> index 8fb5271a95060..1a2361850bd2c 100644
> --- a/drivers/iommu/iommufd/ioas.c
> +++ b/drivers/iommu/iommufd/ioas.c
> @@ -183,6 +183,57 @@ int iommufd_ioas_allow_iovas(struct iommufd_ucmd *ucmd)
>  	return rc;
>  }
>  
> +int iommufd_ioas_reserve_iovas(struct iommufd_ucmd *ucmd)
> +{
> +	struct iommu_ioas_reserve_iovas *cmd = ucmd->cmd;
> +	struct iommu_iova_range __user *ranges;
> +	struct iommufd_ioas *ioas;
> +	u32 i;
> +	int rc;
> +
> +	if (cmd->flags)
> +		return -EOPNOTSUPP;
> +
> +	ioas = iommufd_get_ioas(ucmd->ictx, cmd->ioas_id);
> +	if (IS_ERR(ioas))
> +		return PTR_ERR(ioas);
> +
> +	ranges = u64_to_user_ptr(cmd->ranges);
> +
> +	down_write(&ioas->iopt.iova_rwsem);
> +	for (i = 0; i < cmd->num_ranges; i++) {
> +		struct iommu_iova_range range;
> +
> +		rc = copy_from_user(&range, &ranges[i], sizeof(range));
> +		if (rc) {
> +			rc = -EFAULT;
> +			goto out_unlock;
> +		}
> +		if (range.last < range.start) {
> +			rc = -EINVAL;
> +			goto out_unlock;
> +		}
> +		if (range.start >= ULONG_MAX || range.last >= ULONG_MAX) {
> +			rc = -EOVERFLOW;
> +			goto out_unlock;
> +		}

[Severity: Medium]
Should the check in iommufd_ioas_reserve_iovas() in
drivers/iommu/iommufd/ioas.c use > ULONG_MAX instead of >= ULONG_MAX?

struct iommu_iova_range represents an inclusive [start, last] interval, and
iopt_reserve_iova() supports inclusive bounds up to ULONG_MAX, as seen in
drivers/iommu/iommufd/io_pagetable.c:iopt_table_add_domain():

	if (geometry->aperture_end != ULONG_MAX) {
		rc = iopt_reserve_iova(iopt, geometry->aperture_end + 1,
				       ULONG_MAX, domain);

The UAPI comment for struct iommu_ioas_reserve_iovas in
include/uapi/linux/iommufd.h also notes:

 * A range with last < start fails with -EINVAL.  On 32-bit kernels
 * a range exceeding the native address width fails with -EOVERFLOW.

With >= ULONG_MAX, any range starting or ending at ULONG_MAX (such as a
32-bit PCI host bridge MMIO window ending at 0xffffffff on a 32-bit kernel,
or a reservation reaching U64_MAX on a 64-bit kernel) is rejected with
-EOVERFLOW.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028090453.2392991-1-guanghuifeng@linux.alibaba.com?part=3