On Sat, Jun 25, 2022 at 04:53:25PM +0200, Helge Deller wrote: > On 6/25/22 14:45, Daniel Vetter wrote: > > On Sat, Jun 25, 2022 at 02:24:59PM +0200, Helge Deller wrote: > >> Prevent that users set a font size which is bigger than the physical screen. > >> It's unlikely this may happen (because screens are usually much larger than the > >> fonts and each font char is limited to 32x32 pixels), but it may happen on > >> smaller screens/LCD displays. > >> > >> Signed-off-by: Helge Deller <deller@xxxxxx> > >> Cc: stable@xxxxxxxxxxxxxxx # v4.14+ > >> --- > >> drivers/video/fbdev/core/fbcon.c | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> > >> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c > >> index c4e91715ef00..e162d5e753e5 100644 > >> --- a/drivers/video/fbdev/core/fbcon.c > >> +++ b/drivers/video/fbdev/core/fbcon.c > >> @@ -2469,6 +2469,11 @@ static int fbcon_set_font(struct vc_data *vc, struct console_font *font, > >> if (charcount != 256 && charcount != 512) > >> return -EINVAL; > >> > >> + /* font bigger than screen resolution ? */ > >> + if (font->width > FBCON_SWAP(info->var.rotate, info->var.xres, info->var.yres) || > >> + font->height > FBCON_SWAP(info->var.rotate, info->var.yres, info->var.xres)) > >> + return -EINVAL; > > > > Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > Thanks! > > > Maybe as a safety follow up patch, we have a few copies of the below: > > > > cols = FBCON_SWAP(ops->rotate, info->var.xres, info->var.yres); > > rows = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres); > > cols /= vc->vc_font.width; > > rows /= vc->vc_font.height; > > > > Might be good to extract that into a helper and also add > > > > WARN_ON(!cols); > > WARN_ON(!rows); > > That's not needed then. > The checks I added above will ensure that cols and rows are both greater than 0. Yeah I reviewed it too, but I don't trust review all over the place. Especially with something like fbcon with entry points from everywhere. -Daniel > > to make sure we really didn't screw this up and give syzkaller et all an > > easier time finding bugs - it doesn't need to discover the full exploit, > > only needs to get to this here. > > > > Also maybe even check that cols/rows is within reasons, like smaller than > > BIT(24) or so (so that we have a bunch of headroom for overflows). > > Not needed either. > cols and rows is the screen size divided by an value between 1-32 (the max > font size). So, since screen size is already the higest limit, cols&rows > will always be smaller than screen size (and > 0). > > Helge -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
