| Message ID | alpine.DEB.2.21.2110252317110.58149@angie.orcam.me.uk (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | vgacon: Propagate console boot parameters before calling `vc_resize' | expand |
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.
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;
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 <wim@djo.tudelft.nl> Reported-by: Pavel V. Panteleev <panteleev_p@mcst.ru> 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