Skip to content

Commit d88ca7e

Browse files
Tetsuo Handadanvet
authored andcommitted
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 Reported-and-tested-by: syzbot <[email protected]> Suggested-by: Daniel Vetter <[email protected]> Signed-off-by: Tetsuo Handa <[email protected]> Reported-by: kernel test robot <[email protected]> for missing #include Signed-off-by: Daniel Vetter <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent f369bc3 commit d88ca7e

File tree

4 files changed

+7
-12
lines changed

4 files changed

+7
-12
lines changed

drivers/video/fbdev/core/fbmem.c

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -957,7 +957,6 @@ static int fb_check_caps(struct fb_info *info, struct fb_var_screeninfo *var,
957957
int
958958
fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
959959
{
960-
int flags = info->flags;
961960
int ret = 0;
962961
u32 activate;
963962
struct fb_var_screeninfo old_var;
@@ -1052,9 +1051,6 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
10521051
event.data = &mode;
10531052
fb_notifier_call_chain(FB_EVENT_MODE_CHANGE, &event);
10541053

1055-
if (flags & FBINFO_MISC_USEREVENT)
1056-
fbcon_update_vcs(info, activate & FB_ACTIVATE_ALL);
1057-
10581054
return 0;
10591055
}
10601056
EXPORT_SYMBOL(fb_set_var);
@@ -1105,9 +1101,9 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
11051101
return -EFAULT;
11061102
console_lock();
11071103
lock_fb_info(info);
1108-
info->flags |= FBINFO_MISC_USEREVENT;
11091104
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);
11111107
unlock_fb_info(info);
11121108
console_unlock();
11131109
if (!ret && copy_to_user(argp, &var, sizeof(var)))

drivers/video/fbdev/core/fbsysfs.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,9 @@ static int activate(struct fb_info *fb_info, struct fb_var_screeninfo *var)
9191

9292
var->activate |= FB_ACTIVATE_FORCE;
9393
console_lock();
94-
fb_info->flags |= FBINFO_MISC_USEREVENT;
9594
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);
9797
console_unlock();
9898
if (err)
9999
return err;

drivers/video/fbdev/ps3fb.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include <linux/freezer.h>
3030
#include <linux/uaccess.h>
3131
#include <linux/fb.h>
32+
#include <linux/fbcon.h>
3233
#include <linux/init.h>
3334

3435
#include <asm/cell-regs.h>
@@ -824,12 +825,12 @@ static int ps3fb_ioctl(struct fb_info *info, unsigned int cmd,
824825
var = info->var;
825826
fb_videomode_to_var(&var, vmode);
826827
console_lock();
827-
info->flags |= FBINFO_MISC_USEREVENT;
828828
/* Force, in case only special bits changed */
829829
var.activate |= FB_ACTIVATE_FORCE;
830830
par->new_mode_id = val;
831831
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);
833834
console_unlock();
834835
}
835836
break;

include/linux/fb.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,8 +400,6 @@ struct fb_tile_ops {
400400
#define FBINFO_HWACCEL_YPAN 0x2000 /* optional */
401401
#define FBINFO_HWACCEL_YWRAP 0x4000 /* optional */
402402

403-
#define FBINFO_MISC_USEREVENT 0x10000 /* event request
404-
from userspace */
405403
#define FBINFO_MISC_TILEBLITTING 0x20000 /* use tile blitting */
406404

407405
/* A driver may set this flag to indicate that it does want a set_par to be

0 commit comments

Comments
 (0)