From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Sun, 28 Feb 2016 08:55:46 -0600 From: Alex Williamson To: "Michael S. Tsirkin" Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, Baptiste Reynal , Eric Auger , Antonios Motakis , Julia Lawall , kvm@vger.kernel.org Subject: Re: [PATCH v3 for-4.5] vfio: fix ioctl error handling Message-ID: <20160228085546.45edb552@ul30vt.home> In-Reply-To: <1456669853-19606-1-git-send-email-mst@redhat.com> References: <1456669853-19606-1-git-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: On Sun, 28 Feb 2016 16:31:39 +0200 "Michael S. Tsirkin" wrote: > Calling return copy_to_user(...) in an ioctl will not > do the right thing if there's a pagefault: > copy_to_user returns the number of bytes not copied > in this case. > > Fix up vfio to do > return copy_to_user(...)) ? > -EFAULT : 0; > > everywhere. > > Cc: stable@vger.kernel.org > Signed-off-by: Michael S. Tsirkin > --- Applied to for-linus branch for 4.5. Thanks, Alex > Changes from v2: > use ? : to match desired vfio coding style > Change from v1: > fix up good path return code > > drivers/vfio/pci/vfio_pci.c | 9 ++++++--- > drivers/vfio/platform/vfio_platform_common.c | 9 ++++++--- > drivers/vfio/vfio_iommu_type1.c | 6 ++++-- > 3 files changed, 16 insertions(+), 8 deletions(-) > > diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c > index 2760a7b..8c80a48 100644 > --- a/drivers/vfio/pci/vfio_pci.c > +++ b/drivers/vfio/pci/vfio_pci.c > @@ -446,7 +446,8 @@ static long vfio_pci_ioctl(void *device_data, > info.num_regions = VFIO_PCI_NUM_REGIONS; > info.num_irqs = VFIO_PCI_NUM_IRQS; > > - return copy_to_user((void __user *)arg, &info, minsz); > + return copy_to_user((void __user *)arg, &info, minsz) ? > + -EFAULT : 0; > > } else if (cmd == VFIO_DEVICE_GET_REGION_INFO) { > struct pci_dev *pdev = vdev->pdev; > @@ -520,7 +521,8 @@ static long vfio_pci_ioctl(void *device_data, > return -EINVAL; > } > > - return copy_to_user((void __user *)arg, &info, minsz); > + return copy_to_user((void __user *)arg, &info, minsz) ? > + -EFAULT : 0; > > } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) { > struct vfio_irq_info info; > @@ -555,7 +557,8 @@ static long vfio_pci_ioctl(void *device_data, > else > info.flags |= VFIO_IRQ_INFO_NORESIZE; > > - return copy_to_user((void __user *)arg, &info, minsz); > + return copy_to_user((void __user *)arg, &info, minsz) ? > + -EFAULT : 0; > > } else if (cmd == VFIO_DEVICE_SET_IRQS) { > struct vfio_irq_set hdr; > diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c > index 418cdd9..e65b142 100644 > --- a/drivers/vfio/platform/vfio_platform_common.c > +++ b/drivers/vfio/platform/vfio_platform_common.c > @@ -219,7 +219,8 @@ static long vfio_platform_ioctl(void *device_data, > info.num_regions = vdev->num_regions; > info.num_irqs = vdev->num_irqs; > > - return copy_to_user((void __user *)arg, &info, minsz); > + return copy_to_user((void __user *)arg, &info, minsz) ? > + -EFAULT : 0; > > } else if (cmd == VFIO_DEVICE_GET_REGION_INFO) { > struct vfio_region_info info; > @@ -240,7 +241,8 @@ static long vfio_platform_ioctl(void *device_data, > info.size = vdev->regions[info.index].size; > info.flags = vdev->regions[info.index].flags; > > - return copy_to_user((void __user *)arg, &info, minsz); > + return copy_to_user((void __user *)arg, &info, minsz) ? > + -EFAULT : 0; > > } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) { > struct vfio_irq_info info; > @@ -259,7 +261,8 @@ static long vfio_platform_ioctl(void *device_data, > info.flags = vdev->irqs[info.index].flags; > info.count = vdev->irqs[info.index].count; > > - return copy_to_user((void __user *)arg, &info, minsz); > + return copy_to_user((void __user *)arg, &info, minsz) ? > + -EFAULT : 0; > > } else if (cmd == VFIO_DEVICE_SET_IRQS) { > struct vfio_irq_set hdr; > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index 6f1ea3d..75b24e9 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -999,7 +999,8 @@ static long vfio_iommu_type1_ioctl(void *iommu_data, > > info.iova_pgsizes = vfio_pgsize_bitmap(iommu); > > - return copy_to_user((void __user *)arg, &info, minsz); > + return copy_to_user((void __user *)arg, &info, minsz) ? > + -EFAULT : 0; > > } else if (cmd == VFIO_IOMMU_MAP_DMA) { > struct vfio_iommu_type1_dma_map map; > @@ -1032,7 +1033,8 @@ static long vfio_iommu_type1_ioctl(void *iommu_data, > if (ret) > return ret; > > - return copy_to_user((void __user *)arg, &unmap, minsz); > + return copy_to_user((void __user *)arg, &unmap, minsz) ? > + -EFAULT : 0; > } > > return -ENOTTY;