[PATCH] fbdev: core: Fix pointer desynchronization in fb_io_read()

[PATCH] fbdev: core: Fix pointer desynchronization in fb_io_read()

[Mingyu Wang]

In fb_io_read(), if copy_to_user() performs a partial copy (e.g., due to
a faulty user buffer), the loop adjusts the chunk size 'c' and updates
the remaining 'count'. However, the hardware 'src' pointer has already
been eagerly advanced by the original chunk size.

If the loop is allowed to continue, the read will resume from an
incorrect, over-advanced offset. Since the remaining 'count' was only
decremented by the successful bytes, this desynchronization causes the
next iterations to execute more hardware reads than originally bounded,
eventually leading to out-of-bounds I/O reads.

Fix this by breaking out of the loop immediately upon a partial
copy_to_user(). A partial copy indicates a faulty user buffer, making
subsequent read attempts futile. Breaking out ensures we return the
number of successfully read bytes without risking out-of-bounds hardware
accesses in subsequent mismatched iterations.

Fixes: 6121cd9ef911 ("fbdev: Move I/O read and write code into helper functions")
Cc: stable@vger.kernel.org
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
 drivers/video/fbdev/core/fb_io_fops.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/video/fbdev/core/fb_io_fops.c b/drivers/video/fbdev/core/fb_io_fops.c
index 6ab60fcd0050..0798e88799eb 100644
--- a/drivers/video/fbdev/core/fb_io_fops.c
+++ b/drivers/video/fbdev/core/fb_io_fops.c
@@ -61,6 +61,14 @@ ssize_t fb_io_read(struct fb_info *info, char __user *buf, size_t count, loff_t
 		buf += c;
 		cnt += c;
 		count -= c;
+
+		/*
+		 * If there was a partial copy, the user buffer is faulty.
+		 * Break out to avoid over-advancing the src pointer and
+		 * reading out of bounds in the next iteration.
+		 */
+		if (trailing)
+			break;
 	}
 
 	kfree(buffer);
-- 
2.34.1

Re: [PATCH] fbdev: core: Fix pointer desynchronization in fb_io_read()

[Helge Deller]

On 7/21/26 10:19, Mingyu Wang wrote:
> In fb_io_read(), if copy_to_user() performs a partial copy (e.g., due to
> a faulty user buffer), the loop adjusts the chunk size 'c' and updates
> the remaining 'count'. However, the hardware 'src' pointer has already
> been eagerly advanced by the original chunk size.
> 
> If the loop is allowed to continue, the read will resume from an
> incorrect, over-advanced offset. Since the remaining 'count' was only
> decremented by the successful bytes, this desynchronization causes the
> next iterations to execute more hardware reads than originally bounded,
> eventually leading to out-of-bounds I/O reads.
> 
> Fix this by breaking out of the loop immediately upon a partial
> copy_to_user(). A partial copy indicates a faulty user buffer, making
> subsequent read attempts futile. Breaking out ensures we return the
> number of successfully read bytes without risking out-of-bounds hardware
> accesses in subsequent mismatched iterations.
> 
> Fixes: 6121cd9ef911 ("fbdev: Move I/O read and write code into helper functions")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
> ---
>   drivers/video/fbdev/core/fb_io_fops.c | 8 ++++++++
>   1 file changed, 8 insertions(+)

Thanks for the patch!
It seems correct, and I've added it to the fbdev git tree.

Did you actually ran into a problem, or how did you find this optimization?

Helge

  
> diff --git a/drivers/video/fbdev/core/fb_io_fops.c b/drivers/video/fbdev/core/fb_io_fops.c
> index 6ab60fcd0050..0798e88799eb 100644
> --- a/drivers/video/fbdev/core/fb_io_fops.c
> +++ b/drivers/video/fbdev/core/fb_io_fops.c
> @@ -61,6 +61,14 @@ ssize_t fb_io_read(struct fb_info *info, char __user *buf, size_t count, loff_t
>   		buf += c;
>   		cnt += c;
>   		count -= c;
> +
> +		/*
> +		 * If there was a partial copy, the user buffer is faulty.
> +		 * Break out to avoid over-advancing the src pointer and
> +		 * reading out of bounds in the next iteration.
> +		 */
> +		if (trailing)
> +			break;
>   	}
>   
>   	kfree(buffer);

Re: [PATCH] fbdev: core: Fix pointer desynchronization in fb_io_read()

[Mingyu Wang]

> Thanks for the patch!
> It seems correct, and I've added it to the fbdev git tree.
>
> Did you actually ran into a problem, or how did you find this 
> optimization?
>
>
Hi Helge,

Thanks for reviewing and applying the patch!

To answer your question: this wasn't triggered on production hardware.
It was found while using our DevGen virtual device framework together
with syzkaller to fuzz legacy fbdev drivers.

DevGen triggered several OOB crashes in fbdev, and during the manual
code audit to root-cause them, I noticed the src pointer desynchronization
in fb_io_read(). It was a clear logic bug, so I extracted it as a
standalone fix.

Thanks again for your time and for maintaining the subsystem!

Best regards,
Mingyu Wang