History

fbmem: pull fbcon_update_vcs() out of fb_set_var()

syzbot is reporting OOB read bug in vc_do_resize() [1] caused by memcpy() based on outdated old_{rows,row_size} values, for resize_screen() can recurse into vc_do_resize() which changes vc->vc_{cols,rows} that outdates old_{rows,row_size} values which were saved before calling resize_screen(). Daniel Vetter explained that resize_screen() should not recurse into fbcon_update_vcs() path due to FBINFO_MISC_USEREVENT being still set when calling resize_screen(). Instead of masking FBINFO_MISC_USEREVENT before calling fbcon_update_vcs(), we can remove FBINFO_MISC_USEREVENT by calling fbcon_update_vcs() only if fb_set_var() returned 0. This change assumes that it is harmless to call fbcon_update_vcs() when fb_set_var() returned 0 without reaching fb_notifier_call_chain(). [1] https://syzkaller.appspot.com/bug?id=c70c88cfd16dcf6e1d3c7f0ab8648b3144b5b25e

Tetsuo Handa committed 6 years ago

Commit d88ca7e1 parent f369bc34 files · +7 −12Browse files

Unified · Split

4 files
  1. drivers/video/fbdev/core/fbmem.c +2 −6
  2. drivers/video/fbdev/core/fbsysfs.c +2 −2
  3. drivers/video/fbdev/ps3fb.c +3 −2
  4. include/linux/fb.h +0 −2
drivers/video/fbdev/core/fbmem.c+2−6
@@ -957,7 +957,6 @@ static int fb_check_caps(struct fb_info *info, struct fb_var_screeninfo *var, · 956 unmodified lines
957int957int
958fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)958fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
959{959{
960 int flags = info->flags;
961 int ret = 0;960 int ret = 0;
962 u32 activate;961 u32 activate;
963 struct fb_var_screeninfo old_var;962 struct fb_var_screeninfo old_var;
@@ -1052,9 +1051,6 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var) · 88 unmodified lines
1052 event.data = &mode;1051 event.data = &mode;
1053 fb_notifier_call_chain(FB_EVENT_MODE_CHANGE, &event);1052 fb_notifier_call_chain(FB_EVENT_MODE_CHANGE, &event);
1054 1053
1055 if (flags & FBINFO_MISC_USEREVENT)
1056 fbcon_update_vcs(info, activate & FB_ACTIVATE_ALL);
1057
1058 return 0;1054 return 0;
1059}1055}
1060EXPORT_SYMBOL(fb_set_var);1056EXPORT_SYMBOL(fb_set_var);
@@ -1105,9 +1101,9 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, · 44 unmodified lines
1105 return -EFAULT;1101 return -EFAULT;
1106 console_lock();1102 console_lock();
1107 lock_fb_info(info);1103 lock_fb_info(info);
1108 info->flags |= FBINFO_MISC_USEREVENT;
1109 ret = fb_set_var(info, &var);1104 ret = fb_set_var(info, &var);
1110 info->flags &= ~FBINFO_MISC_USEREVENT;1105 if (!ret)
1106 fbcon_update_vcs(info, var.activate & FB_ACTIVATE_ALL);
1111 unlock_fb_info(info);1107 unlock_fb_info(info);
1112 console_unlock();1108 console_unlock();
1113 if (!ret && copy_to_user(argp, &var, sizeof(var)))1109 if (!ret && copy_to_user(argp, &var, sizeof(var)))
drivers/video/fbdev/core/fbsysfs.c+2−2
@@ -91,9 +91,9 @@ static int activate(struct fb_info *fb_info, struct fb_var_screeninfo *var) · 90 unmodified lines
91 91
92 var->activate |= FB_ACTIVATE_FORCE;92 var->activate |= FB_ACTIVATE_FORCE;
93 console_lock();93 console_lock();
94 fb_info->flags |= FBINFO_MISC_USEREVENT;
95 err = fb_set_var(fb_info, var);94 err = fb_set_var(fb_info, var);
96 fb_info->flags &= ~FBINFO_MISC_USEREVENT;95 if (!err)
96 fbcon_update_vcs(fb_info, var->activate & FB_ACTIVATE_ALL);
97 console_unlock();97 console_unlock();
98 if (err)98 if (err)
99 return err;99 return err;
drivers/video/fbdev/ps3fb.c+3−2
@@ -29,6 +29,7 @@ · 28 unmodified lines
29#include <linux/freezer.h>29#include <linux/freezer.h>
30#include <linux/uaccess.h>30#include <linux/uaccess.h>
31#include <linux/fb.h>31#include <linux/fb.h>
32#include <linux/fbcon.h>
32#include <linux/init.h>33#include <linux/init.h>
33 34
34#include <asm/cell-regs.h>35#include <asm/cell-regs.h>
@@ -824,12 +825,12 @@ static int ps3fb_ioctl(struct fb_info *info, unsigned int cmd, · 789 unmodified lines
824 var = info->var;825 var = info->var;
825 fb_videomode_to_var(&var, vmode);826 fb_videomode_to_var(&var, vmode);
826 console_lock();827 console_lock();
827 info->flags |= FBINFO_MISC_USEREVENT;
828 /* Force, in case only special bits changed */828 /* Force, in case only special bits changed */
829 var.activate |= FB_ACTIVATE_FORCE;829 var.activate |= FB_ACTIVATE_FORCE;
830 par->new_mode_id = val;830 par->new_mode_id = val;
831 retval = fb_set_var(info, &var);831 retval = fb_set_var(info, &var);
832 info->flags &= ~FBINFO_MISC_USEREVENT;832 if (!retval)
833 fbcon_update_vcs(info, var.activate & FB_ACTIVATE_ALL);
833 console_unlock();834 console_unlock();
834 }835 }
835 break;836 break;
include/linux/fb.h+0−2
@@ -400,8 +400,6 @@ struct fb_tile_ops { · 399 unmodified lines
400#define FBINFO_HWACCEL_YPAN 0x2000 /* optional */400#define FBINFO_HWACCEL_YPAN 0x2000 /* optional */
401#define FBINFO_HWACCEL_YWRAP 0x4000 /* optional */401#define FBINFO_HWACCEL_YWRAP 0x4000 /* optional */
402 402
403#define FBINFO_MISC_USEREVENT 0x10000 /* event request
404 from userspace */
405#define FBINFO_MISC_TILEBLITTING 0x20000 /* use tile blitting */403#define FBINFO_MISC_TILEBLITTING 0x20000 /* use tile blitting */
406 404
407/* A driver may set this flag to indicate that it does want a set_par to be405/* A driver may set this flag to indicate that it does want a set_par to be