Re: [PATCH V3 5/9] iommufd: IOMMU_IOAS_MAP_FILE

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Oct 17, 2024 at 01:00:51PM -0400, Steven Sistare wrote:
> On 10/16/2024 8:41 AM, Jason Gunthorpe wrote:
> > On Fri, Oct 04, 2024 at 11:48:16AM -0700, Steve Sistare wrote:
> > > +int iommufd_ioas_map_file(struct iommufd_ucmd *ucmd)
> > > +{
> > > +	struct iommu_ioas_map_file *cmd = ucmd->cmd;
> > > +	unsigned long iova = cmd->iova;
> > > +	struct iommufd_ioas *ioas;
> > > +	unsigned int flags = 0;
> > > +	int rc;
> > > +	struct file *file;
> > > +	unsigned long end;
> > > +
> > > +	if (cmd->iova >= ULONG_MAX || cmd->length >= ULONG_MAX)
> > > +		return -EOVERFLOW;
> > > +
> > > +	if (cmd->length > 0 &&
> > > +	    check_add_overflow(cmd->start, cmd->length - 1, &end))
> > > +		return -EOVERFLOW;
> > 
> > Why is this here? iopt_alloc_pages() does this validation and I don't
> > see math on these values between here and there?
> 
> Since the refactoring of iopt_alloc_pages into iopt_alloc_user_pages and
> iopt_alloc_file_pages, the former checks for overflow of start + length,
> but the latter does not.
>
> To make it symmetric, I could push the check from here (iommufd_ioas_map_file)
> down to iopt_alloc_file_pages.  Or, hoist the check from  iopt_alloc_user_pages to
> iommufd_ioas_map. Your call.

Put the check closer to the actual math it is protecting from overflow
please.

Jason