Re: [PATCH v2] fbdev: fix use-after-free in store_modes()
Helge Deller <[email protected]>
| Newsgroups | org.kernel.vger.linux-fbdev,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 6/26/26 06:50, Ian Bridges wrote:
> store_modes() replaces a framebuffer's modelist with modes from userspace.
> On success it frees the old modelist with fb_destroy_modelist(). Two
> fields still point into that freed list.
>
> One pointer is fb_display[i].mode, the mode a console is using.
> fbcon_new_modelist() moves these pointers to the new list. It only does so
> for consoles still mapped to the framebuffer. An unmapped console is
> skipped and keeps its stale pointer. Unbinding fbcon, for example, sets
> con2fb_map[i] to -1 but leaves fb_display[i].mode set. An
> FBIOPUT_VSCREENINFO ioctl with FB_ACTIVATE_INV_MODE later reaches
> fbcon_mode_deleted(). That function reads the stale fb_display[i].mode
> through fb_mode_is_equal(). The read is a use-after-free.
>
> The other pointer is fb_info->mode, the current mode. It is set through
> the mode sysfs attribute. store_modes() does not update fb_info->mode, so
> it is left pointing into the freed list. show_mode(), the attribute's read
> handler, dereferences the stale fb_info->mode through mode_string(). The
> read is a use-after-free.
>
> Clear both pointers before freeing the list. Commit a1f305893074 ("fbcon:
> Set fb_display[i]->mode to NULL when the mode is released") added the
> helper fbcon_delete_modelist(). It clears every fb_display[i].mode that
> points into a given list. So far it is called only from the unregister
> path. Call it from store_modes() too, and set fb_info->mode to NULL.
>
> Reported-by: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=81c7c6b52649fd07299d
> Cc: [email protected]
> Link: https://lore.kernel.org/all/ajjoDhAi2y4ArSlz@dev/
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Ian Bridges <[email protected]>
> ---
> Added in v2: clear fb_info->mode, which is left dangling by the same free
> in store_modes(). Sashiko flagged that second pointer while reviewing
> v1 [1].
>
> [1] https://lore.kernel.org/all/[email protected]/
>
> drivers/video/fbdev/core/fbsysfs.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
applied.
Thanks!
Helge