fbdev: Return number of bytes read or written

Always return the number of bytes read or written within the
framebuffer. Only return an errno code if framebuffer memory
was not touched. This is the semantics required by POSIX and
makes fb_read() and fb_write() compatible with IGT tests. [1]

This bug has been fixed for fb_write() long ago by
commit 6a2a88668e90 ("[PATCH] fbdev: Fix return error of
fb_write"). The code in fb_read() and the corresponding fb_sys_()
helpers was forgotten.

It can happen that copy_{from, to}_user() only partially copies
the given buffer. Take this into account when calculating the
number of bytes.

v2:
	* consider return value from copy_{from,to}_user() (Geert)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Sui Jingfeng <suijingfeng@loongson.cn>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Helge Deller <deller@gmx.de>
Link: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/fbdev.c
Link: https://patchwork.freedesktop.org/patch/msgid/20230428122452.4856-15-tzimmermann@suse.de

diff --git a/drivers/video/fbdev/core/fb_sys_fops.c b/drivers/video/fbdev/core/fb_sys_fops.c
index ff275d7f3eaf128085aeb7dbdee2826b145fe9d6..cefb77b9546d083074b1d2eb65cf0c13f48932fb 100644 (file)
--- a/drivers/video/fbdev/core/fb_sys_fops.c
+++ b/drivers/video/fbdev/core/fb_sys_fops.c
@@ -19,7 +19,8 @@ ssize_t fb_sys_read(struct fb_info *info, char __user *buf, size_t count,
        unsigned long p = *ppos;
        void *src;
        int err = 0;
-       unsigned long total_size;
+       unsigned long total_size, c;
+       ssize_t ret;
 
        if (info->state != FBINFO_STATE_RUNNING)
                return -EPERM;
@@ -43,13 +44,14 @@ ssize_t fb_sys_read(struct fb_info *info, char __user *buf, size_t count,
        if (info->fbops->fb_sync)
                info->fbops->fb_sync(info);
 
-       if (copy_to_user(buf, src, count))
+       c = copy_to_user(buf, src, count);
+       if (c)
                err = -EFAULT;
+       ret = count - c;
 
-       if  (!err)
-               *ppos += count;
+       *ppos += ret;
 
-       return (err) ? err : count;
+       return ret ? ret : err;
 }
 EXPORT_SYMBOL_GPL(fb_sys_read);
 
@@ -59,7 +61,8 @@ ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
        unsigned long p = *ppos;
        void *dst;
        int err = 0;
-       unsigned long total_size;
+       unsigned long total_size, c;
+       size_t ret;
 
        if (info->state != FBINFO_STATE_RUNNING)
                return -EPERM;
@@ -89,13 +92,14 @@ ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
        if (info->fbops->fb_sync)
                info->fbops->fb_sync(info);
 
-       if (copy_from_user(dst, buf, count))
+       c = copy_from_user(dst, buf, count);
+       if (c)
                err = -EFAULT;
+       ret = count - c;
 
-       if  (!err)
-               *ppos += count;
+       *ppos += ret;
 
-       return (err) ? err : count;
+       return ret ? ret : err;
 }
 EXPORT_SYMBOL_GPL(fb_sys_write);
 

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 875541ff185bf365f2a046e55c2421d50790649a..9c79fb076c6d9059027e8293eb18b9cd18306638 100644 (file)
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -766,7 +766,7 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
        u8 *buffer, *dst;
        u8 __iomem *src;
        int c, cnt = 0, err = 0;
-       unsigned long total_size;
+       unsigned long total_size, trailing;
 
        if (!info || ! info->screen_base)
                return -ENODEV;
@@ -808,10 +808,13 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
                dst += c;
                src += c;
 
-               if (copy_to_user(buf, buffer, c)) {
+               trailing = copy_to_user(buf, buffer, c);
+               if (trailing == c) {
                        err = -EFAULT;
                        break;
                }
+               c -= trailing;
+
                *ppos += c;
                buf += c;
                cnt += c;
@@ -820,7 +823,7 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 
        kfree(buffer);
 
-       return (err) ? err : cnt;
+       return cnt ? cnt : err;
 }
 
 static ssize_t
@@ -831,7 +834,7 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
        u8 *buffer, *src;
        u8 __iomem *dst;
        int c, cnt = 0, err = 0;
-       unsigned long total_size;
+       unsigned long total_size, trailing;
 
        if (!info || !info->screen_base)
                return -ENODEV;
@@ -876,10 +879,12 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
                c = (count > PAGE_SIZE) ? PAGE_SIZE : count;
                src = buffer;
 
-               if (copy_from_user(src, buf, c)) {
+               trailing = copy_from_user(src, buf, c);
+               if (trailing == c) {
                        err = -EFAULT;
                        break;
                }
+               c -= trailing;
 
                fb_memcpy_tofb(dst, src, c);
                dst += c;