From: Daniel Vetter daniel.vetter@ffwll.ch
commit 3c4e0dff2095c579b142d5a0693257f1c58b4804 upstream.
It's buggy:
On Fri, Nov 06, 2020 at 10:30:08PM +0800, Minh Yuan wrote:
We recently discovered a slab-out-of-bounds read in fbcon in the latest kernel ( v5.10-rc2 for now ). The root cause of this vulnerability is that "fbcon_do_set_font" did not handle "vc->vc_font.data" and "vc->vc_font.height" correctly, and the patch https://lkml.org/lkml/2020/9/27/223 for VT_RESIZEX can't handle this issue.
Specifically, we use KD_FONT_OP_SET to set a small font.data for tty6, and use KD_FONT_OP_SET again to set a large font.height for tty1. After that, we use KD_FONT_OP_COPY to assign tty6's vc_font.data to tty1's vc_font.data in "fbcon_do_set_font", while tty1 retains the original larger height. Obviously, this will cause an out-of-bounds read, because we can access a smaller vc_font.data with a larger vc_font.height.
Further there was only one user ever. - Android's loadfont, busybox and console-tools only ever use OP_GET and OP_SET - fbset documentation only mentions the kernel cmdline font: option, not anything else. - systemd used OP_COPY before release 232 published in Nov 2016
Now unfortunately the crucial report seems to have gone down with gmane, and the commit message doesn't say much. But the pull request hints at OP_COPY being broken
https://github.com/systemd/systemd/pull/3651
So in other words, this never worked, and the only project which foolishly every tried to use it, realized that rather quickly too.
Instead of trying to fix security issues here on dead code by adding missing checks, fix the entire thing by removing the functionality.
Note that systemd code using the OP_COPY function ignored the return value, so it doesn't matter what we're doing here really - just in case a lone server somewhere happens to be extremely unlucky and running an affected old version of systemd. The relevant code from font_copy_to_all_vcs() in systemd was:
/* copy font from active VT, where the font was uploaded to */ cfo.op = KD_FONT_OP_COPY; cfo.height = vcs.v_active-1; /* tty1 == index 0 */ (void) ioctl(vcfd, KDFONTOP, &cfo);
Note this just disables the ioctl, garbage collecting the now unused callbacks is left for -next.
v2: Tetsuo found the old mail, which allowed me to find it on another archive. Add the link too.
Acked-by: Peilin Ye yepeilin.cs@gmail.com Reported-by: Minh Yuan yuanmingbuaa@gmail.com Cc: Greg KH greg@kroah.com Cc: Peilin Ye yepeilin.cs@gmail.com Cc: Tetsuo Handa penguin-kernel@i-love.sakura.ne.jp Signed-off-by: Daniel Vetter daniel.vetter@intel.com Link: https://lore.kernel.org/r/20201108153806.3140315-1-daniel.vetter@ffwll.ch Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- drivers/tty/vt/vt.c | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-)
--- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -4700,27 +4700,6 @@ static int con_font_default(struct vc_da return rc; }
-static int con_font_copy(struct vc_data *vc, struct console_font_op *op) -{ - int con = op->height; - int rc; - - - console_lock(); - if (vc->vc_mode != KD_TEXT) - rc = -EINVAL; - else if (!vc->vc_sw->con_font_copy) - rc = -ENOSYS; - else if (con < 0 || !vc_cons_allocated(con)) - rc = -ENOTTY; - else if (con == vc->vc_num) /* nothing to do */ - rc = 0; - else - rc = vc->vc_sw->con_font_copy(vc, con); - console_unlock(); - return rc; -} - int con_font_op(struct vc_data *vc, struct console_font_op *op) { switch (op->op) { @@ -4731,7 +4710,8 @@ int con_font_op(struct vc_data *vc, stru case KD_FONT_OP_SET_DEFAULT: return con_font_default(vc, op); case KD_FONT_OP_COPY: - return con_font_copy(vc, op); + /* was buggy and never really used */ + return -EINVAL; } return -ENOSYS; }