Hi
Am 17.11.25 um 11:32 schrieb Javier Martinez Canillas:
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 ?
I've ran the testing on amdgpu and i915 so that nothing breaks. The bug
is hard to reproduce though. I've discovered it by reading the code.
About Fixes, the problem has been in the code forever. So IDK what Fixes
would make sense. Just in case:
Fixes: 6a9ee8af344e ("vga_switcheroo: initial implementation (v15)")
Cc: Dave Airlie <[email protected]>
Cc: Jani Nikula <[email protected]>
Cc: Joonas Lahtinen <[email protected]>
Cc: Rodrigo Vivi <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Cc: Lyude Paul <[email protected]>
Cc: Danilo Krummrich <[email protected]>
Cc: Alex Deucher <[email protected]>
Cc: "Christian König" <[email protected]>
Cc: Maarten Lankhorst <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: Lukas Wunner <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Helge Deller <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Simona Vetter <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: <[email protected]> # v2.6.34+
Best regards
Thomas
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
