[bug report] iommufd: vfio container FD ioctl compatibility

[Dan Carpenter <error27@gmail.com>]

[ Resending two weeks of email because mutt + msmtp has been silently
  eating my outgoing mail instead of delivering it.  *sigh* -dan ]

Hello Jason Gunthorpe,

The patch 32c328dc9b73: "iommufd: vfio container FD ioctl
compatibility" from Dec 15, 2021, leads to the following Smatch
static checker warning:

	drivers/iommu/iommufd/vfio_compat.c:174 iommufd_vfio_unmap_dma()
	warn: potential integer overflow from user (local copy) 'unmap.iova + unmap.size'

drivers/iommu/iommufd/vfio_compat.c
    140 static int iommufd_vfio_unmap_dma(struct iommufd_ctx *ictx, unsigned int cmd,
    141                                   void __user *arg)
    142 {
    143         size_t minsz = offsetofend(struct vfio_iommu_type1_dma_unmap, size);
    144         /*
    145          * VFIO_DMA_UNMAP_FLAG_GET_DIRTY_BITMAP is obsoleted by the new
    146          * dirty tracking direction:
    147          *  https://lore.kernel.org/kvm/20220731125503.142683-1-yishaih@nvidia.com/
    148          *  https://lore.kernel.org/kvm/20220428210933.3583-1-joao.m.martins@oracle.com/
    149          */
    150         u32 supported_flags = VFIO_DMA_UNMAP_FLAG_ALL;
    151         struct vfio_iommu_type1_dma_unmap unmap;
    152         struct iommufd_ioas *ioas;
    153         unsigned long unmapped;
    154         int rc;
    155 
    156         if (copy_from_user(&unmap, arg, minsz))
    157                 return -EFAULT;
    158 
    159         if (unmap.argsz < minsz || unmap.flags & ~supported_flags)
    160                 return -EINVAL;
    161 
    162         ioas = get_compat_ioas(ictx);
    163         if (IS_ERR(ioas))
    164                 return PTR_ERR(ioas);
    165 
    166         if (unmap.flags & VFIO_DMA_UNMAP_FLAG_ALL) {
    167                 if (unmap.iova != 0 || unmap.size != 0) {
    168                         rc = -EINVAL;
    169                         goto err_put;
    170                 }
    171                 rc = iopt_unmap_all(&ioas->iopt, &unmapped);
    172         } else {
    173                 if (READ_ONCE(ioas->iopt.disable_large_pages)) {
--> 174                         unsigned long iovas[] = { unmap.iova + unmap.size - 1,

The unmap.iova + unmap.size addition can have an integer overflow.  It's
unclear to me if this is caught later or if it has any negative
implications.

What I remeber from looking at similar code is that because of the - 1,
it's supposed to be allowed to overflow to 0 but not further than that.
In other words the highest allowed value it ULONG_MAX and not
"ULONG_MAX - 1".

    175                                                   unmap.iova - 1 };
    176 
    177                         rc = iopt_cut_iova(&ioas->iopt, iovas,
    178                                            unmap.iova ? 2 : 1);
    179                         if (rc)
    180                                 goto err_put;
    181                 }
    182                 rc = iopt_unmap_iova(&ioas->iopt, unmap.iova, unmap.size,
    183                                      &unmapped);
    184         }
    185         unmap.size = unmapped;
    186         if (copy_to_user(arg, &unmap, minsz))
    187                 rc = -EFAULT;
    188 
    189 err_put:
    190         iommufd_put_object(&ioas->obj);
    191         return rc;
    192 }

regards,
dan carpenter