Skip to contentSkip to conversation

fbdev: fbcon: Make sure modelist not set on unregistered console

PATCH Applied Kees Cook 2 participants 2 messages May 9, 2025 – May 12, 2025

Subsystems (3)
lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH 1/1 fbdev: fbcon: Make sure modelist not set on unregistered console

It looks like attempting to write to the "store_modes" sysfs node will run afoul of unregistered consoles:

UBSAN: array-index-out-of-bounds in drivers/video/fbdev/core/fbcon.c:122:28
index -1 is out of range for type 'fb_info *[32]'
...
 fbcon_info_from_console+0x192/0x1a0 drivers/video/fbdev/core/fbcon.c:122
 fbcon_new_modelist+0xbf/0x2d0 drivers/video/fbdev/core/fbcon.c:3048
 fb_new_modelist+0x328/0x440 drivers/video/fbdev/core/fbmem.c:673
 store_modes+0x1c9/0x3e0 drivers/video/fbdev/core/fbsysfs.c:113
 dev_attr_store+0x55/0x80 drivers/base/core.c:2439

static struct fb_info *fbcon_registered_fb[FB_MAX];
...
static signed char con2fb_map[MAX_NR_CONSOLES];
...
static struct fb_info *fbcon_info_from_console(int console)
...

        return fbcon_registered_fb[con2fb_map[console]];

If con2fb_map contains a -1 things go wrong here. Instead, return NULL, as callers of fbcon_info_from_console() are trying to compare against existing "info" pointers, so error handling should kick in correctly.

---
Cc: Simona Vetter <[email protected]>
Cc: Helge Deller <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: "Jiri Slaby (SUSE)" <[email protected]>
Cc: Javier Martinez Canillas <[email protected]>
Cc: Shixiong Ou <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>

---
 drivers/video/fbdev/core/fbcon.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/video/fbdev/core/fbcon.c +6 -1
index ac3c99ed92d1..2df48037688d 100644
@@ -117,9 +117,14 @@ static signed char con2fb_map_boot[MAX_NR_CONSOLES];
 
 static struct fb_info *fbcon_info_from_console(int console)
 {
+	signed char fb;
 	WARN_CONSOLE_UNLOCKED();
 
-	return fbcon_registered_fb[con2fb_map[console]];
+	fb = con2fb_map[console];
+	if (fb < 0 || fb >= ARRAY_SIZE(fbcon_registered_fb))
+		return NULL;
+
+	return fbcon_registered_fb[fb];
 }
 
 static int logo_lines;
Signature · 1 line
2.34.1
maintainer Applied
#1 lore raw
Quoted 24 lines · Kees Cook

It looks like attempting to write to the "store_modes" sysfs node will run afoul of unregistered consoles:

UBSAN: array-index-out-of-bounds in drivers/video/fbdev/core/fbcon.c:122:28
index -1 is out of range for type 'fb_info *[32]'
...
  fbcon_info_from_console+0x192/0x1a0 drivers/video/fbdev/core/fbcon.c:122
  fbcon_new_modelist+0xbf/0x2d0 drivers/video/fbdev/core/fbcon.c:3048
  fb_new_modelist+0x328/0x440 drivers/video/fbdev/core/fbmem.c:673
  store_modes+0x1c9/0x3e0 drivers/video/fbdev/core/fbsysfs.c:113
  dev_attr_store+0x55/0x80 drivers/base/core.c:2439

static struct fb_info *fbcon_registered_fb[FB_MAX];
...
static signed char con2fb_map[MAX_NR_CONSOLES];
...
static struct fb_info *fbcon_info_from_console(int console)
...

         return fbcon_registered_fb[con2fb_map[console]];

If con2fb_map contains a -1 things go wrong here. Instead, return NULL, as callers of fbcon_info_from_console() are trying to compare against existing "info" pointers, so error handling should kick in correctly.

Quoted 6 lines
---
  drivers/video/fbdev/core/fbcon.c | 7 ++++++-
  1 file changed, 6 insertions(+), 1 deletion(-)

applied to fbdev for-next git tree.

Thanks!
Helge