Re: [bug report] vfio: hugepage support for vfio_iommu_type1

[Dan Carpenter <dan.carpenter@oracle.com>]

On Thu, Oct 12, 2017 at 10:32:41AM -0600, Alex Williamson wrote:
> Hi Dan,
> 
> On Thu, 12 Oct 2017 15:40:36 +0300
> Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> > Hello Alex Williamson,
> > 
> > The patch 166fd7d94afd: "vfio: hugepage support for vfio_iommu_type1"
> > from Jun 21, 2013, leads to the following static checker warning:
> > 
> > 	drivers/vfio/vfio_iommu_type1.c:819 vfio_dma_do_unmap()
> > 	warn: overflowed symbol reused:  'unmap->size'
> > 
> > drivers/vfio/vfio_iommu_type1.c
> >    756  static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
> >    757                               struct vfio_iommu_type1_dma_unmap *unmap)
> >    758  {
> >    759          uint64_t mask;
> >    760          struct vfio_dma *dma, *dma_last = NULL;
> >    761          size_t unmapped = 0;
> >    762          int ret = 0, retries = 0;
> >    763  
> >    764          mask = ((uint64_t)1 << __ffs(vfio_pgsize_bitmap(iommu))) - 1;
> >    765  
> >    766          if (unmap->iova & mask)
> >    767                  return -EINVAL;
> >    768          if (!unmap->size || unmap->size & mask)
> >    769                  return -EINVAL;
> > 
> > unmap is controlled by the user from the ioctl.  ->iova and ->size are
> > type u64.  It would be simple enough to add an integer overflow check
> > here, but I'd be tempted to do something like:
> > 
> > 		if (unmap->size + unmap->iova < unmap->size ||
> > 		    unmap->size + unmap->iova > SIZE_MAX)
> > 			return -EINVAL;
> > 
> > Because below we are passing it to vfio_find_dma() which only takes a
> > size_t and not a u64.
> 
> Ok, but why test unmap->iova + unmap->size relative to SIZE_MAX rather
> than just unmap->size since we use a dma_addr_t for the iova in
> vfio_find_dma()?

Oh, yeah, you're right.  I misread the code a bit.  I'll send a check
for that then.

regards,
dan carpenter