Re: [PATCH v2 1/2] fbdev: bound mode sysfs output to the sysfs buffer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Lockless reads of `fb_info->var` cause torn reads and state corruption.
--

commit b5317fd1a6886d886e344587e27b5ae1721b2542
Author: Melbin K Mathew <mlbnkm1@gmail.com>

fbdev: bound mode sysfs output to the sysfs buffer
    
mode_string() uses snprintf() which can return a value larger than the
remaining buffer space. show_modes() accumulates the return value into i
without checking whether i has reached PAGE_SIZE. This commit adds a size
parameter to mode_string() and uses scnprintf() to return only the bytes
actually written.
    
Cc: stable@vger.kernel.org

> diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c
> index ea196603c7a87..af21dc5052df3 100644
> --- a/drivers/video/fbdev/core/fbsysfs.c
> +++ b/drivers/video/fbdev/core/fbsysfs.c

[ ... ]

> @@ -64,7 +67,7 @@ static ssize_t store_mode(struct device *device, struct device_attribute *attr,
>  
>  	list_for_each_entry(modelist, &fb_info->modelist, list) {
>  		mode = &modelist->mode;
> -		i = mode_string(mstr, 0, mode);
> +		i = mode_string(mstr, sizeof(mstr), 0, mode);
>  		if (strncmp(mstr, buf, max(count, i)) == 0) {
>  
>  			var = fb_info->var;

[Severity: High]
This isn't a bug introduced by this patch, but while looking at fbsysfs.c,
I noticed several other functions perform a lockless copy of the
fb_info->var struct:

    var = fb_info->var;

This happens in store_bpp(), store_rotate(), store_virtual(), and store_pan().

If this lockless sysfs read races with an ioctl(FBIOPUT_VSCREENINFO)
executing fb_set_var() (which modifies info->var under fb_info->lock),
could it tear the large struct fb_var_screeninfo?

Modifying and writing this corrupted state back to the hardware via activate()
or fb_pan_display() might configure the display controller with invalid 
settings, potentially causing display corruption or crashes in low-level 
graphics drivers.

Should these other functions also serialize their access with lock_fb_info()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701221757.231490-1-mlbnkm1@gmail.com?part=1