[PATCH] vgacon: Propagate console boot parameters before calling `vc_resize'

STALE1811d LANDED

Landed in mainline as 3dfac26e2ef2 on 2021-11-25.

2 messages, 2 authors, 2021-10-27 · open the first message on its own page

[PATCH] vgacon: Propagate console boot parameters before calling `vc_resize'

From: "Maciej W. Rozycki" <macro@orcam.me.uk>
Date: 2021-10-25 22:26:36

Fix a division by zero in `vgacon_resize' with a backtrace like:

vgacon_resize
vc_do_resize
vgacon_init
do_bind_con_driver
do_unbind_con_driver
fbcon_fb_unbind
do_unregister_framebuffer
do_register_framebuffer
register_framebuffer
__drm_fb_helper_initial_config_and_unlock
drm_helper_hpd_irq_event
dw_hdmi_irq
irq_thread
kthread

caused by `c->vc_cell_height' not having been initialized.  This has 
only started to trigger with commit 860dafa90259 ("vt: Fix character 
height handling with VT_RESIZEX"), however the ultimate offender is 
commit 50ec42edd978 ("[PATCH] Detaching fbcon: fix vgacon to allow 
retaking of the console").

Said commit has added a call to `vc_resize' whenever `vgacon_init' is 
called with the `init' argument set to 0, which did not happen before. 
And the call is made before a key vgacon boot parameter retrieved in 
`vgacon_startup' has been propagated in `vgacon_init' for `vc_resize' to 
use to the console structure being worked on.  Previously the parameter 
was `c->vc_font.height' and now it is `c->vc_cell_height'.

In this particular scenario the registration of fbcon has failed and vt 
resorts to vgacon.  Now fbcon does have initialized `c->vc_font.height' 
somehow, unlike `c->vc_cell_height', which is why this code did not 
crash before, but either way the boot parameters should have been copied 
to the console structure ahead of the call to `vc_resize' rather than 
afterwards, so that first the call has a chance to use them and second 
they do not change the console structure to something possibly different 
from what was used by `vc_resize'.

Move the propagation of the vgacon boot parameters ahead of the call to 
`vc_resize' then.  Adjust the comment accordingly.

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Reported-by: Wim Osterholt <redacted>
Reported-by: Pavel V. Panteleev <redacted>
Fixes: 50ec42edd978 ("[PATCH] Detaching fbcon: fix vgacon to allow retaking of the console")
Cc: stable@vger.kernel.org # v2.6.18+
---
 drivers/video/console/vgacon.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

linux-vt-vgacon-init-cell-height-fix.diff
Index: linux-macro-ide-tty/drivers/video/console/vgacon.c
===================================================================
--- linux-macro-ide-tty.orig/drivers/video/console/vgacon.c
+++ linux-macro-ide-tty/drivers/video/console/vgacon.c
@@ -366,11 +366,17 @@ static void vgacon_init(struct vc_data *
 	struct uni_pagedir *p;
 
 	/*
-	 * We cannot be loaded as a module, therefore init is always 1,
-	 * but vgacon_init can be called more than once, and init will
-	 * not be 1.
+	 * We cannot be loaded as a module, therefore init will be 1
+	 * if we are the default console, however if we are a fallback
+	 * console, for example if fbcon has failed registration, then
+	 * init will be 0, so we need to make sure our boot parameters
+	 * have been copied to the console structure for vgacon_resize
+	 * ultimately called by vc_resize.  Any subsequent calls to
+	 * vgacon_init init will have init set to 0 too.
 	 */
 	c->vc_can_do_color = vga_can_do_color;
+	c->vc_scan_lines = vga_scan_lines;
+	c->vc_font.height = c->vc_cell_height = vga_video_font_height;
 
 	/* set dimensions manually if init != 0 since vc_resize() will fail */
 	if (init) {
@@ -379,8 +385,6 @@ static void vgacon_init(struct vc_data *
 	} else
 		vc_resize(c, vga_video_num_columns, vga_video_num_lines);
 
-	c->vc_scan_lines = vga_scan_lines;
-	c->vc_font.height = c->vc_cell_height = vga_video_font_height;
 	c->vc_complement_mask = 0x7700;
 	if (vga_512_chars)
 		c->vc_hi_font_mask = 0x0800;

Re: [PATCH] vgacon: Propagate console boot parameters before calling `vc_resize'

From: wim <hidden>
Date: 2021-10-27 00:55:47

On Tue, Oct 26, 2021 at 12:26:22AM +0200, Maciej W. Rozycki wrote:
quoted hunk
Fix a division by zero in `vgacon_resize' with a backtrace like:
...
linux-vt-vgacon-init-cell-height-fix.diff
Index: linux-macro-ide-tty/drivers/video/console/vgacon.c
===================================================================
--- linux-macro-ide-tty.orig/drivers/video/console/vgacon.c
+++ linux-macro-ide-tty/drivers/video/console/vgacon.c
@@ -366,11 +366,17 @@ static void vgacon_init(struct vc_data *
 	struct uni_pagedir *p;
 
 	/*
-	 * We cannot be loaded as a module, therefore init is always 1,
-	 * but vgacon_init can be called more than once, and init will
-	 * not be 1.
+	 * We cannot be loaded as a module, therefore init will be 1
+	 * if we are the default console, however if we are a fallback
+	 * console, for example if fbcon has failed registration, then
+	 * init will be 0, so we need to make sure our boot parameters
+	 * have been copied to the console structure for vgacon_resize
+	 * ultimately called by vc_resize.  Any subsequent calls to
+	 * vgacon_init init will have init set to 0 too.
 	 */
 	c->vc_can_do_color = vga_can_do_color;
+	c->vc_scan_lines = vga_scan_lines;
+	c->vc_font.height = c->vc_cell_height = vga_video_font_height;
 
 	/* set dimensions manually if init != 0 since vc_resize() will fail */
 	if (init) {
@@ -379,8 +385,6 @@ static void vgacon_init(struct vc_data *
 	} else
 		vc_resize(c, vga_video_num_columns, vga_video_num_lines);
 
-	c->vc_scan_lines = vga_scan_lines;
-	c->vc_font.height = c->vc_cell_height = vga_video_font_height;
 	c->vc_complement_mask = 0x7700;
 	if (vga_512_chars)
 		c->vc_hi_font_mask = 0x0800;

Just to let you know that the above patch fixes the crashes I experienced.
Tried in kernel 4.9.287.
The git tree is broken in the sense that it doesn't know the tags v4.9.284,
v4.9.285, v4.9.286 and v2.4.287.

Wim.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help