Re: [PATCHv3 6/9] staging: vme_user: switch to returning -EFAULT on __copy_*_user errors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 26/06/15 21:39, Dmitry Kalinkin wrote:
Signed-off-by: Dmitry Kalinkin <dmitry.kalinkin@xxxxxxxxx>
---
  drivers/staging/vme/devices/vme_user.c | 47 ++++++++--------------------------
  1 file changed, 11 insertions(+), 36 deletions(-)

diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c
index a2345db..ef876a4 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -123,7 +123,6 @@ struct vme_user_vma_priv {
  static ssize_t resource_to_user(int minor, char __user *buf, size_t count,
  				loff_t *ppos)
  {
-	ssize_t retval;
  	ssize_t copied = 0;

  	if (count > image[minor].size_buf)
@@ -135,13 +134,8 @@ static ssize_t resource_to_user(int minor, char __user *buf, size_t count,
  	if (copied < 0)
  		return (int)copied;

-	retval = __copy_to_user(buf, image[minor].kern_buf,
-				(unsigned long)copied);
-	if (retval != 0) {
-		copied = (copied - retval);
-		pr_info("User copy failed\n");
-		return -EINVAL;
-	}
+	if (__copy_to_user(buf, image[minor].kern_buf, (unsigned long)copied))
+		return -EFAULT;

  	return copied;

Does __copy_to_user() not return the number of bytes still to be copied? The above seems to add the assumption that __copy_to_user()
can't return part way through a copy.

  }
@@ -149,21 +143,16 @@ static ssize_t resource_to_user(int minor, char __user *buf, size_t count,
  static ssize_t resource_from_user(unsigned int minor, const char __user *buf,
  				  size_t count, loff_t *ppos)
  {
-	ssize_t retval;
  	ssize_t copied = 0;

  	if (count > image[minor].size_buf)
  		count = image[minor].size_buf;

-	retval = __copy_from_user(image[minor].kern_buf, buf,
-				  (unsigned long)count);
-	if (retval != 0)
-		copied = (copied - retval);
-	else
-		copied = count;
+	if (__copy_from_user(image[minor].kern_buf, buf, (unsigned long)count))
+		return -EFAULT;


Same here.

  	copied = vme_master_write(image[minor].resource, image[minor].kern_buf,
-				  copied, *ppos);
+				  count, *ppos);

  	return copied;
  }
@@ -172,38 +161,24 @@ static ssize_t buffer_to_user(unsigned int minor, char __user *buf,
  			      size_t count, loff_t *ppos)
  {
  	void *image_ptr;
-	ssize_t retval;

  	image_ptr = image[minor].kern_buf + *ppos;
+	if (__copy_to_user(buf, image_ptr, (unsigned long)count))
+		return -EFAULT;


Ditto.

-	retval = __copy_to_user(buf, image_ptr, (unsigned long)count);
-	if (retval != 0) {
-		retval = (count - retval);
-		pr_warn("Partial copy to userspace\n");
-	} else
-		retval = count;
-
-	/* Return number of bytes successfully read */
-	return retval;
+	return count;
  }

  static ssize_t buffer_from_user(unsigned int minor, const char __user *buf,
  				size_t count, loff_t *ppos)
  {
  	void *image_ptr;
-	size_t retval;

  	image_ptr = image[minor].kern_buf + *ppos;
+	if (__copy_from_user(image_ptr, buf, (unsigned long)count))
+		return -EFAULT;


And here.

-	retval = __copy_from_user(image_ptr, buf, (unsigned long)count);
-	if (retval != 0) {
-		retval = (count - retval);
-		pr_warn("Partial copy to userspace\n");
-	} else
-		retval = count;
-
-	/* Return number of bytes successfully read */
-	return retval;
+	return count;
  }

  static ssize_t vme_user_read(struct file *file, char __user *buf, size_t count,


--
Martyn Welch (Lead Software Engineer)  | Registered in England and Wales
GE Intelligent Platforms               | (3828642) at 100 Barbirolli Square
T +44(0)1327322748                     | Manchester, M2 3AB
E martyn.welch@xxxxxx                  | VAT:GB 927559189
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux