Re: [PATCH] drm, fbcon, vga_switcheroo: Avoid race condition in fbcon setup

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 17.11.25 um 11:32 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> 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 <tzimmermann@suse.de>
>> 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 <javierm@redhat.com>
>
> 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 <airlied@redhat.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Tvrtko Ursulin <tursulin@ursulin.net>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Helge Deller <deller@gmx.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Cc: linux-fbdev@vger.kernel.org
Cc: <stable@vger.kernel.org> # 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)