fbcon_do_set_font() calls vc_resize() when font size is changed.
However, if if vc_resize() failed, current implementation doesn't
revert changes for font size, and this causes inconsistent state.
syzbot reported unable to handle page fault due to this issue [1].
syzbot's repro uses fault injection which cause failure for memory
allocation, so vc_resize() failed.
This patch fixes this issue by properly revert changes for font
related date when vc_resize() failed.
Link: https://syzkaller.appspot.com/bug?id=3443d3a1fa6d964dd7310a0cb1696d165a3e07c4 [1]
Reported-by: [email protected]
Signed-off-by: Shigeru Yoshida <[email protected]>
---
drivers/video/fbdev/core/fbcon.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index cf9ac4da0a82..825b012debe7 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2401,15 +2401,21 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
struct fb_info *info = fbcon_info_from_console(vc->vc_num);
struct fbcon_ops *ops = info->fbcon_par;
struct fbcon_display *p = &fb_display[vc->vc_num];
- int resize;
+ int resize, ret, old_userfont, old_width, old_height, old_charcount;
char *old_data = NULL;
resize = (w != vc->vc_font.width) || (h != vc->vc_font.height);
if (p->userfont)
old_data = vc->vc_font.data;
vc->vc_font.data = (void *)(p->fontdata = data);
+ old_userfont = p->userfont;
if ((p->userfont = userfont))
REFCOUNT(data)++;
+
+ old_width = vc->vc_font.width;
+ old_height = vc->vc_font.height;
+ old_charcount = vc->vc_font.charcount;
+
vc->vc_font.width = w;
vc->vc_font.height = h;
vc->vc_font.charcount = charcount;
@@ -2425,7 +2431,9 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
rows = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres);
cols /= w;
rows /= h;
- vc_resize(vc, cols, rows);
+ ret = vc_resize(vc, cols, rows);
+ if (ret)
+ goto err_out;
} else if (con_is_visible(vc)
&& vc->vc_mode == KD_TEXT) {
fbcon_clear_margins(vc, 0);
@@ -2435,6 +2443,21 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
if (old_data && (--REFCOUNT(old_data) == 0))
kfree(old_data - FONT_EXTRA_WORDS * sizeof(int));
return 0;
+
+err_out:
+ p->fontdata = old_data;
+ vc->vc_font.data = (void *)old_data;
+
+ if (userfont) {
+ p->userfont = old_userfont;
+ REFCOUNT(data)--;
+ }
+
+ vc->vc_font.width = old_width;
+ vc->vc_font.height = old_height;
+ vc->vc_font.charcount = old_charcount;
+
+ return ret;
}
/*
--
2.37.1
Hi Shigeru,
On 8/18/22 20:13, Shigeru Yoshida wrote:
> fbcon_do_set_font() calls vc_resize() when font size is changed.
> However, if if vc_resize() failed, current implementation doesn't
> revert changes for font size, and this causes inconsistent state.
>
> syzbot reported unable to handle page fault due to this issue [1].
> syzbot's repro uses fault injection which cause failure for memory
> allocation, so vc_resize() failed.
>
> This patch fixes this issue by properly revert changes for font
> related date when vc_resize() failed.
>
> Link: https://syzkaller.appspot.com/bug?id=3443d3a1fa6d964dd7310a0cb1696d165a3e07c4 [1]
> Reported-by: [email protected]
> Signed-off-by: Shigeru Yoshida <[email protected]>
applied.
Thank you!
Helge
> ---
> drivers/video/fbdev/core/fbcon.c | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index cf9ac4da0a82..825b012debe7 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -2401,15 +2401,21 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
> struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_ops *ops = info->fbcon_par;
> struct fbcon_display *p = &fb_display[vc->vc_num];
> - int resize;
> + int resize, ret, old_userfont, old_width, old_height, old_charcount;
> char *old_data = NULL;
>
> resize = (w != vc->vc_font.width) || (h != vc->vc_font.height);
> if (p->userfont)
> old_data = vc->vc_font.data;
> vc->vc_font.data = (void *)(p->fontdata = data);
> + old_userfont = p->userfont;
> if ((p->userfont = userfont))
> REFCOUNT(data)++;
> +
> + old_width = vc->vc_font.width;
> + old_height = vc->vc_font.height;
> + old_charcount = vc->vc_font.charcount;
> +
> vc->vc_font.width = w;
> vc->vc_font.height = h;
> vc->vc_font.charcount = charcount;
> @@ -2425,7 +2431,9 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
> rows = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres);
> cols /= w;
> rows /= h;
> - vc_resize(vc, cols, rows);
> + ret = vc_resize(vc, cols, rows);
> + if (ret)
> + goto err_out;
> } else if (con_is_visible(vc)
> && vc->vc_mode == KD_TEXT) {
> fbcon_clear_margins(vc, 0);
> @@ -2435,6 +2443,21 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
> if (old_data && (--REFCOUNT(old_data) == 0))
> kfree(old_data - FONT_EXTRA_WORDS * sizeof(int));
> return 0;
> +
> +err_out:
> + p->fontdata = old_data;
> + vc->vc_font.data = (void *)old_data;
> +
> + if (userfont) {
> + p->userfont = old_userfont;
> + REFCOUNT(data)--;
> + }
> +
> + vc->vc_font.width = old_width;
> + vc->vc_font.height = old_height;
> + vc->vc_font.charcount = old_charcount;
> +
> + return ret;
> }
>
> /*