Skip to contentSkip to conversation

vfio/pci: return -EFAULT if copy_to_user fails

PATCH Applied Dan Carpenter 3 participants 6 messages Feb 25, 2016 – Feb 28, 2016

Subsystems (3)
KVM Kernel janitors VFIO driver
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 vfio/pci: return -EFAULT if copy_to_user fails

The copy_to_user() function returns the number of bytes that were not copied but we want to return -EFAULT on error here.

Fixes 188ad9d6cbbc ('vfio/pci: Include sparse mmap capability for MSI-X table regions')
Signed-off-by Dan Carpenter <[email protected]>
drivers/vfio/pci/vfio_pci.c +4 -5
index 1ce1d36..98059df 100644
@@ -664,12 +664,11 @@ static long vfio_pci_ioctl(void *device_data,
 				info.cap_offset = 0;
 			} else {
 				vfio_info_cap_shift(&caps, sizeof(info));
-				ret = copy_to_user((void __user *)arg +
-						   sizeof(info), caps.buf,
-						   caps.size);
-				if (ret) {
+				if (copy_to_user((void __user *)arg +
+						  sizeof(info), caps.buf,
+						  caps.size)) {
 					kfree(caps.buf);
-					return ret;
+					return -EFAULT;
 				}
 				info.cap_offset = sizeof(info);
 			}
#1 lore raw
Quoted 4 lines · Dan Carpenter

The copy_to_user() function returns the number of bytes that were not copied but we want to return -EFAULT on error here.

Fixes 188ad9d6cbbc ('vfio/pci: Include sparse mmap capability for MSI-X table regions')

Where's this commit? In which tree?

Quoted 23 lines
Signed-off-by Dan Carpenter <[email protected]>
drivers/vfio/pci/vfio_pci.c +4 -5
index 1ce1d36..98059df 100644
@@ -664,12 +664,11 @@ static long vfio_pci_ioctl(void *device_data,
 				info.cap_offset = 0;
 			} else {
 				vfio_info_cap_shift(&caps, sizeof(info));
-				ret = copy_to_user((void __user *)arg +
-						   sizeof(info), caps.buf,
-						   caps.size);
-				if (ret) {
+				if (copy_to_user((void __user *)arg +
+						  sizeof(info), caps.buf,
+						  caps.size)) {
 					kfree(caps.buf);
-					return ret;
+					return -EFAULT;
 				}
 				info.cap_offset = sizeof(info);
 			}
author
#2 lore raw
Quoted 7 lines · Michael S. Tsirkin
Quoted 4 lines · Dan Carpenter

The copy_to_user() function returns the number of bytes that were not copied but we want to return -EFAULT on error here.

Fixes 188ad9d6cbbc ('vfio/pci: Include sparse mmap capability for MSI-X table regions')

Where's this commit? In which tree?

linux-next. It only has Alex's signed off by. So likely my patch will be folded in?

regards,
dan carpenter

#3 lore raw
Quoted 7 lines · Dan Carpenter
Quoted 6 lines · Michael S. Tsirkin
Quoted 4 lines · Dan Carpenter

The copy_to_user() function returns the number of bytes that were not copied but we want to return -EFAULT on error here.

Fixes 188ad9d6cbbc ('vfio/pci: Include sparse mmap capability for MSI-X table regions')
Quoted 7 lines
Quoted 1 line

Where's this commit? In which tree?

linux-next. It only has Alex's signed off by. So likely my patch will be folded in?

regards,
dan carpenter

BTW, vfio repeats this in otherplaces.
I'll fix it up.

Signature · 1 line
MST
Acked-by on v1 1/1
#4 lore raw
In reply to Dan Carpenter
Quoted 5 lines · Dan Carpenter

The copy_to_user() function returns the number of bytes that were not copied but we want to return -EFAULT on error here.

Fixes 188ad9d6cbbc ('vfio/pci: Include sparse mmap capability for MSI-X table regions')
Signed-off-by Dan Carpenter <[email protected]>
Acked-by Michael S. Tsirkin <[email protected]>

I posted a patch to fix up all other places.

Quoted 21 lines
drivers/vfio/pci/vfio_pci.c +4 -5
index 1ce1d36..98059df 100644
@@ -664,12 +664,11 @@ static long vfio_pci_ioctl(void *device_data,
 				info.cap_offset = 0;
 			} else {
 				vfio_info_cap_shift(&caps, sizeof(info));
-				ret = copy_to_user((void __user *)arg +
-						   sizeof(info), caps.buf,
-						   caps.size);
-				if (ret) {
+				if (copy_to_user((void __user *)arg +
+						  sizeof(info), caps.buf,
+						  caps.size)) {
 					kfree(caps.buf);
-					return ret;
+					return -EFAULT;
 				}
 				info.cap_offset = sizeof(info);
 			}
Applied
#5 lore raw
In reply to Dan Carpenter
Quoted 17 lines · On Thu, 25 Feb 2016 10:52:12 +0300 Dan Carpenter <[email protected]>…

The copy_to_user() function returns the number of bytes that were not copied but we want to return -EFAULT on error here.

Fixes 188ad9d6cbbc ('vfio/pci: Include sparse mmap capability for MSI-X table regions')
Signed-off-by Dan Carpenter <[email protected]>
drivers/vfio/pci/vfio_pci.c +0 -3
index 1ce1d36..98059df 100644
@@ -664,12 +664,11 @@ static long vfio_pci_ioctl(void *device_data,
 				info.cap_offset = 0;
 			} else {
 				vfio_info_cap_shift(&caps, sizeof(info));
-				ret = copy_to_user((void __user *)arg +
-						   sizeof(info), caps.buf,
-						   caps.size);

drivers/vfio/pci/vfio_pci.c static long vfio_pci_ioctl(void *device_data, · line 674

Quoted 10 lines
drivers/vfio/pci/vfio_pci.c +4 -2
-				if (ret) {
+				if (copy_to_user((void __user *)arg +
+						  sizeof(info), caps.buf,
+						  caps.size)) {
 					kfree(caps.buf);
-					return ret;
+					return -EFAULT;
 				}
 				info.cap_offset = sizeof(info);
 			}

Applied to next. Thanks,

Alex