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

Pre-existing issues:
- [High] fb_info->mode can become a dangling pointer if the active mode is 
deleted via ioctl(FBIOPUT_VSCREENINFO) with FB_ACTIVATE_INV_MODE, leading to a 
Use-After-Free in show_mode().
--

commit 243c82e865602245e21ae16b7cdd7738ccfa6a53
Author: Melbin K Mathew <[email protected]>

fbdev: serialize mode sysfs access with lock_fb_info()

Take lock_fb_info() in show_mode(), show_modes(), and store_mode() to
serialize with store_modes(). In show_mode(), copy the mode to the
stack and format after dropping the lock. In store_mode(), split
activate() into a _locked variant to avoid double-locking.

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

[ ... ]

> @@ -85,11 +103,20 @@ static ssize_t show_mode(struct device *device, struct 
> device_attribute *attr,
>                        char *buf)
>  {
>       struct fb_info *fb_info = dev_get_drvdata(device);
> +     struct fb_videomode mode;
> +     bool have_mode = false;
>  
> -     if (!fb_info->mode)
> +     lock_fb_info(fb_info);
> +     if (fb_info->mode) {
> +             mode = *fb_info->mode;

[Severity: High]
This is a pre-existing issue, but can fb_info->mode become a dangling pointer
and lead to a use-after-free here?

If the active mode is deleted via ioctl(FBIOPUT_VSCREENINFO) with
FB_ACTIVATE_INV_MODE, it appears fb_delete_videomode() unlinks and frees
the mode node:

drivers/video/fbdev/core/modedb.c:fb_delete_videomode() {
    ...
    list_del(pos);
    kfree(pos);
    ...
}

However, it does not seem to clear the fb_info->mode reference. If a user
subsequently reads /sys/class/graphics/fb0/mode, would this dereference the
freed memory even while holding the lock?

> +             have_mode = true;
> +     }
> +     unlock_fb_info(fb_info);
> +
> +     if (!have_mode)
>               return 0;
>  
> -     return mode_string(buf, PAGE_SIZE, 0, fb_info->mode);
> +     return mode_string(buf, PAGE_SIZE, 0, &mode);
>  }

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

Reply via email to