lkml.org 
[lkml]   [2015]   [Jul]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCHv3 6/9] staging: vme_user: switch to returning -EFAULT on __copy_*_user errors
On 26/06/15 21:39, Dmitry Kalinkin wrote:
> Signed-off-by: Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
> ---
> 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@ge.com | VAT:GB 927559189


\
 
 \ /
  Last update: 2015-07-06 15:01    [W:0.104 / U:0.800 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site