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 <[email protected]>
Reported-by: Wim Osterholt <[email protected]>
Reported-by: Pavel V. Panteleev <[email protected]>
Fixes: 50ec42edd978 ("[PATCH] Detaching fbcon: fix vgacon to allow retaking of the console")
Cc: [email protected] # 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;
On Tue, Oct 26, 2021 at 12:26:22AM +0200, Maciej W. Rozycki wrote:
> 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.