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

Pre-existing issues:
- [High] Unprotected value copy of large `fb_info->var` struct leads to torn 
reads and state corruption in sysfs store functions.
--

commit 03248859e392cd00678961d93e5c27717ac23498
Author: Melbin K Mathew <[email protected]>

fbdev: bound mode sysfs output to the sysfs buffer

This commit updates mode_string() to take a size parameter and use scnprintf()
to prevent buffer overruns when generating sysfs output. It also modifies
show_modes() to stop accumulating mode strings once the sysfs buffer is full.

> 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 is a pre-existing issue, but seeing the struct copy here prompted a
question about other sysfs store functions in this file.

In store_mode(), var = fb_info->var; is protected by lock_fb_info(fb_info)
acquired earlier in the function. However, other store functions in this file
perform this same large struct copy locklessly before acquiring the lock:

drivers/video/fbdev/core/fbsysfs.c:store_bpp() {
    ...
    var = fb_info->var;
    ...
}

Similar unprotected copies occur in store_rotate(), store_virtual(), and
store_pan().

Can these unprotected copies lead to torn reads if executed concurrently with
an ioctl or another sysfs write that modifies the display state?

If a torn read occurs and a single field is updated before passing the torn
structure to activate(), could this commit corrupted state back to the
hardware?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to