Thomas Zimmermann <[email protected]> writes: Hello Thomas,
> Protect vga_switcheroo_client_fb_set() with console lock. Avoids OOB > access in fbcon_remap_all(). Without holding the console lock the call > races with switching outputs. > > VGA switcheroo calls fbcon_remap_all() when switching clients. The fbcon > function uses struct fb_info.node, which is set by register_framebuffer(). > As the fb-helper code currently sets up VGA switcheroo before registering > the framebuffer, the value of node is -1 and therefore not a legal value. > For example, fbcon uses the value within set_con2fb_map() [1] as an index > into an array. > > Moving vga_switcheroo_client_fb_set() after register_framebuffer() can > result in VGA switching that does not switch fbcon correctly. > > Therefore move vga_switcheroo_client_fb_set() under fbcon_fb_registered(), > which already holds the console lock. Fbdev calls fbcon_fb_registered() > from within register_framebuffer(). Serializes the helper with VGA > switcheroo's call to fbcon_remap_all(). > > Although vga_switcheroo_client_fb_set() takes an instance of struct fb_info > as parameter, it really only needs the contained fbcon state. Moving the > call to fbcon initialization is therefore cleaner than before. Only amdgpu, > i915, nouveau and radeon support vga_switcheroo. For all other drivers, > this change does nothing. > > Signed-off-by: Thomas Zimmermann <[email protected]> > Link: > https://elixir.bootlin.com/linux/v6.17/source/drivers/video/fbdev/core/fbcon.c#L2942 > # [1] > --- I'm not that familiar with fbcon and vga_switcheroo to properly review your patch but after reading the explanation in the commit message and reading the diff, the change does make sense to me. Acked-by: Javier Martinez Canillas <[email protected]> But I think that would be good if you get some testing for the drivers that make use of vga_switcheroo. Also, do you need a Fixes tag ? -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
