ACK: [SRU Groovy,Focal/linux-oem-5.6] vt: Disable KD_FONT_OP_COPY

[Kleber Souza]

On 03.12.20 22:23, Thadeu Lima de Souza Cascardo wrote:
> From: Daniel Vetter <daniel.vetter at ffwll.ch>
> 
> 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 at gmail.com>
> Reported-by: Minh Yuan <yuanmingbuaa at gmail.com>
> References: https://lists.freedesktop.org/archives/systemd-devel/2016-June/036935.html
> References: https://github.com/systemd/systemd/pull/3651
> Cc: Greg KH <greg at kroah.com>
> Cc: Peilin Ye <yepeilin.cs at gmail.com>
> Cc: Tetsuo Handa <penguin-kernel at i-love.sakura.ne.jp>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> Link: https://lore.kernel.org/r/20201108153806.3140315-1-daniel.vetter@ffwll.ch
> Signed-off-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> (cherry picked from commit 3c4e0dff2095c579b142d5a0693257f1c58b4804)
> CVE-2020-28974
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>

Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>

> ---
>   drivers/tty/vt/vt.c | 24 ++----------------------
>   1 file changed, 2 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index bb0623d60cc0..74e3945e7146 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -4643,27 +4643,6 @@ static int con_font_default(struct vc_data *vc, struct console_font_op *op)
>   	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) {
> @@ -4674,7 +4653,8 @@ int con_font_op(struct vc_data *vc, struct console_font_op *op)
>   	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;
>   }
>