Re: [PATCH v2 for-4.5] vfio: fix ioctl error handling
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2016-02-28 14:55:23
Also in:
kvm, lkml
On Sun, Feb 28, 2016 at 08:33:50AM -0600, Alex Williamson wrote:
On Sun, 28 Feb 2016 16:25:07 +0200 "Michael S. Tsirkin" [off-list ref] wrote:quoted
On Sun, Feb 28, 2016 at 08:23:36AM -0600, Alex Williamson wrote:quoted
On Sun, 28 Feb 2016 15:51:49 +0200 "Michael S. Tsirkin" [off-list ref] wrote:quoted
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 if (copy_to_user(...)) return -EFAULT; return 0; everywhere. Cc: stable@vger.kernel.org Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- Changes from v1: fix up good path logic to return 0. Compiled only. drivers/vfio/pci/vfio_pci.c | 12 +++++++++--- drivers/vfio/platform/vfio_platform_common.c | 12 +++++++++--- drivers/vfio/vfio_iommu_type1.c | 8 ++++++-- 3 files changed, 24 insertions(+), 8 deletions(-)diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index 2760a7b..286b91f 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c@@ -446,7 +446,9 @@ 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); + if (copy_to_user((void __user *)arg, &info, minsz)) + return -EFAULT; + return 0;Any objection to a ternary in each case? return copy_to_user((void __user *)arg, &info, minsz) ? -EFAULT : 0;If you prefer, I'll do it in v3.Looks clean and compact to me. Thanks, Alex
Does v3 I posted address your comment? -- MST