show_mode() dereferences fb_info->mode and show_modes() walks fb_info->modelist without holding lock_fb_info(). store_modes() takes lock_fb_info() while replacing the modelist and freeing the old one.
A concurrent reader can load a pointer to an old modelist entry before store_modes() frees it, then dereference freed memory in mode_string(). Take lock_fb_info() in both show_mode() and show_modes() to serialize with store_modes(). In show_mode(), copy the mode to the stack and format the stack copy after dropping the lock. Impact: local kernel UAF read when a privileged writer races with sysfs readers of /sys/class/graphics/fb*/mode and modes. Cc: [email protected] Signed-off-by: Melbin K Mathew <[email protected]> --- A userspace reproducer triggering the race is available to maintainers on request. drivers/video/fbdev/core/fbsysfs.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c index ea196603c7..6bdb25f7be 100644 --- a/drivers/video/fbdev/core/fbsysfs.c +++ b/drivers/video/fbdev/core/fbsysfs.c @@ -82,11 +82,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; + have_mode = true; + } + unlock_fb_info(fb_info); + + if (!have_mode) return 0; - return mode_string(buf, 0, fb_info->mode); + return mode_string(buf, 0, &mode); } static ssize_t store_modes(struct device *device, @@ -134,10 +143,13 @@ static ssize_t show_modes(struct device *device, struct device_attribute *attr, const struct fb_videomode *mode; i = 0; + lock_fb_info(fb_info); list_for_each_entry(modelist, &fb_info->modelist, list) { mode = &modelist->mode; i += mode_string(buf, i, mode); } + unlock_fb_info(fb_info); + return i; } -- 2.39.5
