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: stable@vger.kernel.org
Signed-off-by: Melbin K Mathew <redacted>
---
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