From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Kroah-Hartman Date: Thu, 24 Sep 2020 14:09:37 +0000 Subject: Re: [PATCH 0/3] Prevent out-of-bounds access for built-in font data buffers Message-Id: <20200924140937.GA749208@kroah.com> List-Id: References: <0000000000006b9e8d059952095e@google.com> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Peilin Ye , Daniel Vetter Cc: linux-fbdev@vger.kernel.org, Bartlomiej Zolnierkiewicz , Jiri Slaby , syzkaller-bugs@googlegroups.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-kernel-mentees@lists.linuxfoundation.org On Thu, Sep 24, 2020 at 09:38:22AM -0400, Peilin Ye wrote: > Hi all, > > syzbot has reported [1] a global out-of-bounds read issue in > fbcon_get_font(). A malicious user may resize `vc_font.height` to a large > value in vt_ioctl(), causing fbcon_get_font() to overflow our built-in > font data buffers, declared in lib/fonts/font_*.c: > > (e.g. lib/fonts/font_8x8.c) > #define FONTDATAMAX 2048 > > static const unsigned char fontdata_8x8[FONTDATAMAX] = { > > /* 0 0x00 '^@' */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > [...] > > In order to perform a reliable range check, fbcon_get_font() needs to know > `FONTDATAMAX` for each built-in font under lib/fonts/. Unfortunately, we > do not keep that information in our font descriptor, > `struct console_font`: > > (include/uapi/linux/kd.h) > struct console_font { > unsigned int width, height; /* font size */ > unsigned int charcount; > unsigned char *data; /* font data with height fixed to 32 */ > }; > > To make things worse, `struct console_font` is part of the UAPI, so we > cannot add a new field to keep track of `FONTDATAMAX`. > > Fortunately, the framebuffer layer itself gives us a hint of how to > resolve this issue without changing UAPI. When allocating a buffer for a > user-provided font, fbcon_set_font() reserves four "extra words" at the > beginning of the buffer: > > (drivers/video/fbdev/core/fbcon.c) > new_data = kmalloc(FONT_EXTRA_WORDS * sizeof(int) + size, GFP_USER); > [...] > new_data += FONT_EXTRA_WORDS * sizeof(int); > FNTSIZE(new_data) = size; > FNTCHARCNT(new_data) = charcount; > REFCOUNT(new_data) = 0; /* usage counter */ > [...] > FNTSUM(new_data) = csum; > > Later, to get the size of a data buffer, the framebuffer layer simply > calls FNTSIZE() on it: > > (drivers/video/fbdev/core/fbcon.h) > /* Font */ > #define REFCOUNT(fd) (((int *)(fd))[-1]) > #define FNTSIZE(fd) (((int *)(fd))[-2]) > #define FNTCHARCNT(fd) (((int *)(fd))[-3]) > #define FNTSUM(fd) (((int *)(fd))[-4]) > #define FONT_EXTRA_WORDS 4 > > Currently, this is only done for user-provided fonts. Let us do the same > thing for built-in fonts, prepend these "extra words" (including > `FONTDATAMAX`) to their data buffers, so that other subsystems, like the > framebuffer layer, can use these macros on all fonts, no matter built-in > or user-provided. As an example, this series fixes the syzbot issue in > fbcon_get_font(): > > (drivers/video/fbdev/core/fbcon.c) > if (font->width <= 8) { > j = vc->vc_font.height; > + if (font->charcount * j > FNTSIZE(fontdata)) > + return -EINVAL; > [...] > > Similarly, newport_con also use these macros. It only uses three of them: > > (drivers/video/console/newport_con.c) > /* borrowed from fbcon.c */ > #define REFCOUNT(fd) (((int *)(fd))[-1]) > #define FNTSIZE(fd) (((int *)(fd))[-2]) > #define FNTCHARCNT(fd) (((int *)(fd))[-3]) > #define FONT_EXTRA_WORDS 3 > > To keep things simple, move all these macro definitions to , > use four words instead of three, and initialize the fourth word in > newport_set_font() properly. > > Many thanks to Greg Kroah-Hartman , who > reviewed and improved this series! > > [1]: KASAN: global-out-of-bounds Read in fbcon_get_font > https://syzkaller.appspot.com/bug?idb8be45afea11888776f897895aef9ad1c3ecfd > > Peilin Ye (3): > fbdev, newport_con: Move FONT_EXTRA_WORDS macros into linux/font.h > Fonts: Support FONT_EXTRA_WORDS macros for built-in fonts > fbcon: Fix global-out-of-bounds read in fbcon_get_font() > > drivers/video/console/newport_con.c | 7 +------ > drivers/video/fbdev/core/fbcon.c | 12 ++++++++++++ > drivers/video/fbdev/core/fbcon.h | 7 ------- > drivers/video/fbdev/core/fbcon_rotate.c | 1 + > drivers/video/fbdev/core/tileblit.c | 1 + > include/linux/font.h | 13 +++++++++++++ > lib/fonts/font_10x18.c | 9 ++++----- > lib/fonts/font_6x10.c | 9 +++++---- > lib/fonts/font_6x11.c | 9 ++++----- > lib/fonts/font_7x14.c | 9 ++++----- > lib/fonts/font_8x16.c | 9 ++++----- > lib/fonts/font_8x8.c | 9 ++++----- > lib/fonts/font_acorn_8x8.c | 9 ++++++--- > lib/fonts/font_mini_4x6.c | 8 ++++---- > lib/fonts/font_pearl_8x8.c | 9 ++++----- > lib/fonts/font_sun12x22.c | 9 ++++----- > lib/fonts/font_sun8x16.c | 7 ++++--- > lib/fonts/font_ter16x32.c | 9 ++++----- > 18 files changed, 79 insertions(+), 67 deletions(-) Gotta love going backwards in arrays :) Nice work, whole series is: Reviewed-by: Greg Kroah-Hartman Daniel, can you take this through your tree? thanks, greg k-h From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 34A03C4363D for ; Thu, 24 Sep 2020 14:09:48 +0000 (UTC) Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9056C212CC for ; Thu, 24 Sep 2020 14:09:47 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="lyWX0elQ" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9056C212CC Authentication-Results: mail.kernel.org; dmarc=pass (p=none dis=none) header.from=linuxfoundation.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linux-kernel-mentees-bounces@lists.linuxfoundation.org Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 253358544C; Thu, 24 Sep 2020 14:09:47 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id m_e-uUZbt4YX; Thu, 24 Sep 2020 14:09:46 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id B199784D2E; Thu, 24 Sep 2020 14:09:23 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 9BB43C0889; Thu, 24 Sep 2020 14:09:23 +0000 (UTC) Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id BE755C0051 for ; Thu, 24 Sep 2020 14:09:22 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id AB2FB85725 for ; Thu, 24 Sep 2020 14:09:22 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id YFZfR9UASy9M for ; Thu, 24 Sep 2020 14:09:21 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.7.6 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by whitealder.osuosl.org (Postfix) with ESMTPS id E167F84FBB for ; Thu, 24 Sep 2020 14:09:21 +0000 (UTC) Received: from localhost (83-86-74-64.cable.dynamic.v4.ziggo.nl [83.86.74.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id DFD84212CC; Thu, 24 Sep 2020 14:09:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600956561; bh=G6GAHIyBNjn81+SgTSJpR9h4ZD7LE1n0OOabPWRwiGQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=lyWX0elQq8N8yRcV8TJsOnLALF3IIBbyEitypy5ApDt2JuJ+Vt1TElaFEU3iCm6iP l2p77nbU/n3wRSYNCKUzhnt3EvhwfuWUCjSBFZTd8lcUO/N/RjEgmF6iCde21nhnhL wilEXDU789knoAMyTNbvmRiD3p9hqR/5Rcp3JFyo= Date: Thu, 24 Sep 2020 16:09:37 +0200 From: Greg Kroah-Hartman To: Peilin Ye , Daniel Vetter Message-ID: <20200924140937.GA749208@kroah.com> References: <0000000000006b9e8d059952095e@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Cc: linux-fbdev@vger.kernel.org, Bartlomiej Zolnierkiewicz , Jiri Slaby , syzkaller-bugs@googlegroups.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: Re: [Linux-kernel-mentees] [PATCH 0/3] Prevent out-of-bounds access for built-in font data buffers X-BeenThere: linux-kernel-mentees@lists.linuxfoundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-kernel-mentees-bounces@lists.linuxfoundation.org Sender: "Linux-kernel-mentees" On Thu, Sep 24, 2020 at 09:38:22AM -0400, Peilin Ye wrote: > Hi all, > > syzbot has reported [1] a global out-of-bounds read issue in > fbcon_get_font(). A malicious user may resize `vc_font.height` to a large > value in vt_ioctl(), causing fbcon_get_font() to overflow our built-in > font data buffers, declared in lib/fonts/font_*.c: > > (e.g. lib/fonts/font_8x8.c) > #define FONTDATAMAX 2048 > > static const unsigned char fontdata_8x8[FONTDATAMAX] = { > > /* 0 0x00 '^@' */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > [...] > > In order to perform a reliable range check, fbcon_get_font() needs to know > `FONTDATAMAX` for each built-in font under lib/fonts/. Unfortunately, we > do not keep that information in our font descriptor, > `struct console_font`: > > (include/uapi/linux/kd.h) > struct console_font { > unsigned int width, height; /* font size */ > unsigned int charcount; > unsigned char *data; /* font data with height fixed to 32 */ > }; > > To make things worse, `struct console_font` is part of the UAPI, so we > cannot add a new field to keep track of `FONTDATAMAX`. > > Fortunately, the framebuffer layer itself gives us a hint of how to > resolve this issue without changing UAPI. When allocating a buffer for a > user-provided font, fbcon_set_font() reserves four "extra words" at the > beginning of the buffer: > > (drivers/video/fbdev/core/fbcon.c) > new_data = kmalloc(FONT_EXTRA_WORDS * sizeof(int) + size, GFP_USER); > [...] > new_data += FONT_EXTRA_WORDS * sizeof(int); > FNTSIZE(new_data) = size; > FNTCHARCNT(new_data) = charcount; > REFCOUNT(new_data) = 0; /* usage counter */ > [...] > FNTSUM(new_data) = csum; > > Later, to get the size of a data buffer, the framebuffer layer simply > calls FNTSIZE() on it: > > (drivers/video/fbdev/core/fbcon.h) > /* Font */ > #define REFCOUNT(fd) (((int *)(fd))[-1]) > #define FNTSIZE(fd) (((int *)(fd))[-2]) > #define FNTCHARCNT(fd) (((int *)(fd))[-3]) > #define FNTSUM(fd) (((int *)(fd))[-4]) > #define FONT_EXTRA_WORDS 4 > > Currently, this is only done for user-provided fonts. Let us do the same > thing for built-in fonts, prepend these "extra words" (including > `FONTDATAMAX`) to their data buffers, so that other subsystems, like the > framebuffer layer, can use these macros on all fonts, no matter built-in > or user-provided. As an example, this series fixes the syzbot issue in > fbcon_get_font(): > > (drivers/video/fbdev/core/fbcon.c) > if (font->width <= 8) { > j = vc->vc_font.height; > + if (font->charcount * j > FNTSIZE(fontdata)) > + return -EINVAL; > [...] > > Similarly, newport_con also use these macros. It only uses three of them: > > (drivers/video/console/newport_con.c) > /* borrowed from fbcon.c */ > #define REFCOUNT(fd) (((int *)(fd))[-1]) > #define FNTSIZE(fd) (((int *)(fd))[-2]) > #define FNTCHARCNT(fd) (((int *)(fd))[-3]) > #define FONT_EXTRA_WORDS 3 > > To keep things simple, move all these macro definitions to , > use four words instead of three, and initialize the fourth word in > newport_set_font() properly. > > Many thanks to Greg Kroah-Hartman , who > reviewed and improved this series! > > [1]: KASAN: global-out-of-bounds Read in fbcon_get_font > https://syzkaller.appspot.com/bug?id=08b8be45afea11888776f897895aef9ad1c3ecfd > > Peilin Ye (3): > fbdev, newport_con: Move FONT_EXTRA_WORDS macros into linux/font.h > Fonts: Support FONT_EXTRA_WORDS macros for built-in fonts > fbcon: Fix global-out-of-bounds read in fbcon_get_font() > > drivers/video/console/newport_con.c | 7 +------ > drivers/video/fbdev/core/fbcon.c | 12 ++++++++++++ > drivers/video/fbdev/core/fbcon.h | 7 ------- > drivers/video/fbdev/core/fbcon_rotate.c | 1 + > drivers/video/fbdev/core/tileblit.c | 1 + > include/linux/font.h | 13 +++++++++++++ > lib/fonts/font_10x18.c | 9 ++++----- > lib/fonts/font_6x10.c | 9 +++++---- > lib/fonts/font_6x11.c | 9 ++++----- > lib/fonts/font_7x14.c | 9 ++++----- > lib/fonts/font_8x16.c | 9 ++++----- > lib/fonts/font_8x8.c | 9 ++++----- > lib/fonts/font_acorn_8x8.c | 9 ++++++--- > lib/fonts/font_mini_4x6.c | 8 ++++---- > lib/fonts/font_pearl_8x8.c | 9 ++++----- > lib/fonts/font_sun12x22.c | 9 ++++----- > lib/fonts/font_sun8x16.c | 7 ++++--- > lib/fonts/font_ter16x32.c | 9 ++++----- > 18 files changed, 79 insertions(+), 67 deletions(-) Gotta love going backwards in arrays :) Nice work, whole series is: Reviewed-by: Greg Kroah-Hartman Daniel, can you take this through your tree? thanks, greg k-h _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 40C45C4727D for ; Thu, 24 Sep 2020 14:09:24 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C8D612220C for ; Thu, 24 Sep 2020 14:09:23 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="lyWX0elQ" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C8D612220C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linuxfoundation.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2213E88E84; Thu, 24 Sep 2020 14:09:23 +0000 (UTC) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by gabe.freedesktop.org (Postfix) with ESMTPS id CDD5988E84 for ; Thu, 24 Sep 2020 14:09:21 +0000 (UTC) Received: from localhost (83-86-74-64.cable.dynamic.v4.ziggo.nl [83.86.74.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id DFD84212CC; Thu, 24 Sep 2020 14:09:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600956561; bh=G6GAHIyBNjn81+SgTSJpR9h4ZD7LE1n0OOabPWRwiGQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=lyWX0elQq8N8yRcV8TJsOnLALF3IIBbyEitypy5ApDt2JuJ+Vt1TElaFEU3iCm6iP l2p77nbU/n3wRSYNCKUzhnt3EvhwfuWUCjSBFZTd8lcUO/N/RjEgmF6iCde21nhnhL wilEXDU789knoAMyTNbvmRiD3p9hqR/5Rcp3JFyo= Date: Thu, 24 Sep 2020 16:09:37 +0200 From: Greg Kroah-Hartman To: Peilin Ye , Daniel Vetter Subject: Re: [PATCH 0/3] Prevent out-of-bounds access for built-in font data buffers Message-ID: <20200924140937.GA749208@kroah.com> References: <0000000000006b9e8d059952095e@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-fbdev@vger.kernel.org, Bartlomiej Zolnierkiewicz , Jiri Slaby , syzkaller-bugs@googlegroups.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-kernel-mentees@lists.linuxfoundation.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Thu, Sep 24, 2020 at 09:38:22AM -0400, Peilin Ye wrote: > Hi all, > > syzbot has reported [1] a global out-of-bounds read issue in > fbcon_get_font(). A malicious user may resize `vc_font.height` to a large > value in vt_ioctl(), causing fbcon_get_font() to overflow our built-in > font data buffers, declared in lib/fonts/font_*.c: > > (e.g. lib/fonts/font_8x8.c) > #define FONTDATAMAX 2048 > > static const unsigned char fontdata_8x8[FONTDATAMAX] = { > > /* 0 0x00 '^@' */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > [...] > > In order to perform a reliable range check, fbcon_get_font() needs to know > `FONTDATAMAX` for each built-in font under lib/fonts/. Unfortunately, we > do not keep that information in our font descriptor, > `struct console_font`: > > (include/uapi/linux/kd.h) > struct console_font { > unsigned int width, height; /* font size */ > unsigned int charcount; > unsigned char *data; /* font data with height fixed to 32 */ > }; > > To make things worse, `struct console_font` is part of the UAPI, so we > cannot add a new field to keep track of `FONTDATAMAX`. > > Fortunately, the framebuffer layer itself gives us a hint of how to > resolve this issue without changing UAPI. When allocating a buffer for a > user-provided font, fbcon_set_font() reserves four "extra words" at the > beginning of the buffer: > > (drivers/video/fbdev/core/fbcon.c) > new_data = kmalloc(FONT_EXTRA_WORDS * sizeof(int) + size, GFP_USER); > [...] > new_data += FONT_EXTRA_WORDS * sizeof(int); > FNTSIZE(new_data) = size; > FNTCHARCNT(new_data) = charcount; > REFCOUNT(new_data) = 0; /* usage counter */ > [...] > FNTSUM(new_data) = csum; > > Later, to get the size of a data buffer, the framebuffer layer simply > calls FNTSIZE() on it: > > (drivers/video/fbdev/core/fbcon.h) > /* Font */ > #define REFCOUNT(fd) (((int *)(fd))[-1]) > #define FNTSIZE(fd) (((int *)(fd))[-2]) > #define FNTCHARCNT(fd) (((int *)(fd))[-3]) > #define FNTSUM(fd) (((int *)(fd))[-4]) > #define FONT_EXTRA_WORDS 4 > > Currently, this is only done for user-provided fonts. Let us do the same > thing for built-in fonts, prepend these "extra words" (including > `FONTDATAMAX`) to their data buffers, so that other subsystems, like the > framebuffer layer, can use these macros on all fonts, no matter built-in > or user-provided. As an example, this series fixes the syzbot issue in > fbcon_get_font(): > > (drivers/video/fbdev/core/fbcon.c) > if (font->width <= 8) { > j = vc->vc_font.height; > + if (font->charcount * j > FNTSIZE(fontdata)) > + return -EINVAL; > [...] > > Similarly, newport_con also use these macros. It only uses three of them: > > (drivers/video/console/newport_con.c) > /* borrowed from fbcon.c */ > #define REFCOUNT(fd) (((int *)(fd))[-1]) > #define FNTSIZE(fd) (((int *)(fd))[-2]) > #define FNTCHARCNT(fd) (((int *)(fd))[-3]) > #define FONT_EXTRA_WORDS 3 > > To keep things simple, move all these macro definitions to , > use four words instead of three, and initialize the fourth word in > newport_set_font() properly. > > Many thanks to Greg Kroah-Hartman , who > reviewed and improved this series! > > [1]: KASAN: global-out-of-bounds Read in fbcon_get_font > https://syzkaller.appspot.com/bug?id=08b8be45afea11888776f897895aef9ad1c3ecfd > > Peilin Ye (3): > fbdev, newport_con: Move FONT_EXTRA_WORDS macros into linux/font.h > Fonts: Support FONT_EXTRA_WORDS macros for built-in fonts > fbcon: Fix global-out-of-bounds read in fbcon_get_font() > > drivers/video/console/newport_con.c | 7 +------ > drivers/video/fbdev/core/fbcon.c | 12 ++++++++++++ > drivers/video/fbdev/core/fbcon.h | 7 ------- > drivers/video/fbdev/core/fbcon_rotate.c | 1 + > drivers/video/fbdev/core/tileblit.c | 1 + > include/linux/font.h | 13 +++++++++++++ > lib/fonts/font_10x18.c | 9 ++++----- > lib/fonts/font_6x10.c | 9 +++++---- > lib/fonts/font_6x11.c | 9 ++++----- > lib/fonts/font_7x14.c | 9 ++++----- > lib/fonts/font_8x16.c | 9 ++++----- > lib/fonts/font_8x8.c | 9 ++++----- > lib/fonts/font_acorn_8x8.c | 9 ++++++--- > lib/fonts/font_mini_4x6.c | 8 ++++---- > lib/fonts/font_pearl_8x8.c | 9 ++++----- > lib/fonts/font_sun12x22.c | 9 ++++----- > lib/fonts/font_sun8x16.c | 7 ++++--- > lib/fonts/font_ter16x32.c | 9 ++++----- > 18 files changed, 79 insertions(+), 67 deletions(-) Gotta love going backwards in arrays :) Nice work, whole series is: Reviewed-by: Greg Kroah-Hartman Daniel, can you take this through your tree? thanks, greg k-h _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7B872C4363D for ; Thu, 24 Sep 2020 14:09:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 34F89221EB for ; Thu, 24 Sep 2020 14:09:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600956563; bh=G6GAHIyBNjn81+SgTSJpR9h4ZD7LE1n0OOabPWRwiGQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=wxKuP1Sf+fHTKhM/xt7iY7g/HUWLAsJFsRAj2zw9HADGaoC1yAmWFPnLObpDJVyZH n+vCFGVWgbCiuQMTGV+c/bNyQcuZRw63FCAvqmgu8gVwAt1sJjs5FDz4kR3ss/A6Tn X2MDgf2p0wcN57So8J5b6VS8Wath5KODl6uHZQ34= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728230AbgIXOJW (ORCPT ); Thu, 24 Sep 2020 10:09:22 -0400 Received: from mail.kernel.org ([198.145.29.99]:40108 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728064AbgIXOJW (ORCPT ); Thu, 24 Sep 2020 10:09:22 -0400 Received: from localhost (83-86-74-64.cable.dynamic.v4.ziggo.nl [83.86.74.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id DFD84212CC; Thu, 24 Sep 2020 14:09:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600956561; bh=G6GAHIyBNjn81+SgTSJpR9h4ZD7LE1n0OOabPWRwiGQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=lyWX0elQq8N8yRcV8TJsOnLALF3IIBbyEitypy5ApDt2JuJ+Vt1TElaFEU3iCm6iP l2p77nbU/n3wRSYNCKUzhnt3EvhwfuWUCjSBFZTd8lcUO/N/RjEgmF6iCde21nhnhL wilEXDU789knoAMyTNbvmRiD3p9hqR/5Rcp3JFyo= Date: Thu, 24 Sep 2020 16:09:37 +0200 From: Greg Kroah-Hartman To: Peilin Ye , Daniel Vetter Cc: Bartlomiej Zolnierkiewicz , Jiri Slaby , dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org, linux-kernel-mentees@lists.linuxfoundation.org, syzkaller-bugs@googlegroups.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH 0/3] Prevent out-of-bounds access for built-in font data buffers Message-ID: <20200924140937.GA749208@kroah.com> References: <0000000000006b9e8d059952095e@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Sep 24, 2020 at 09:38:22AM -0400, Peilin Ye wrote: > Hi all, > > syzbot has reported [1] a global out-of-bounds read issue in > fbcon_get_font(). A malicious user may resize `vc_font.height` to a large > value in vt_ioctl(), causing fbcon_get_font() to overflow our built-in > font data buffers, declared in lib/fonts/font_*.c: > > (e.g. lib/fonts/font_8x8.c) > #define FONTDATAMAX 2048 > > static const unsigned char fontdata_8x8[FONTDATAMAX] = { > > /* 0 0x00 '^@' */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > 0x00, /* 00000000 */ > [...] > > In order to perform a reliable range check, fbcon_get_font() needs to know > `FONTDATAMAX` for each built-in font under lib/fonts/. Unfortunately, we > do not keep that information in our font descriptor, > `struct console_font`: > > (include/uapi/linux/kd.h) > struct console_font { > unsigned int width, height; /* font size */ > unsigned int charcount; > unsigned char *data; /* font data with height fixed to 32 */ > }; > > To make things worse, `struct console_font` is part of the UAPI, so we > cannot add a new field to keep track of `FONTDATAMAX`. > > Fortunately, the framebuffer layer itself gives us a hint of how to > resolve this issue without changing UAPI. When allocating a buffer for a > user-provided font, fbcon_set_font() reserves four "extra words" at the > beginning of the buffer: > > (drivers/video/fbdev/core/fbcon.c) > new_data = kmalloc(FONT_EXTRA_WORDS * sizeof(int) + size, GFP_USER); > [...] > new_data += FONT_EXTRA_WORDS * sizeof(int); > FNTSIZE(new_data) = size; > FNTCHARCNT(new_data) = charcount; > REFCOUNT(new_data) = 0; /* usage counter */ > [...] > FNTSUM(new_data) = csum; > > Later, to get the size of a data buffer, the framebuffer layer simply > calls FNTSIZE() on it: > > (drivers/video/fbdev/core/fbcon.h) > /* Font */ > #define REFCOUNT(fd) (((int *)(fd))[-1]) > #define FNTSIZE(fd) (((int *)(fd))[-2]) > #define FNTCHARCNT(fd) (((int *)(fd))[-3]) > #define FNTSUM(fd) (((int *)(fd))[-4]) > #define FONT_EXTRA_WORDS 4 > > Currently, this is only done for user-provided fonts. Let us do the same > thing for built-in fonts, prepend these "extra words" (including > `FONTDATAMAX`) to their data buffers, so that other subsystems, like the > framebuffer layer, can use these macros on all fonts, no matter built-in > or user-provided. As an example, this series fixes the syzbot issue in > fbcon_get_font(): > > (drivers/video/fbdev/core/fbcon.c) > if (font->width <= 8) { > j = vc->vc_font.height; > + if (font->charcount * j > FNTSIZE(fontdata)) > + return -EINVAL; > [...] > > Similarly, newport_con also use these macros. It only uses three of them: > > (drivers/video/console/newport_con.c) > /* borrowed from fbcon.c */ > #define REFCOUNT(fd) (((int *)(fd))[-1]) > #define FNTSIZE(fd) (((int *)(fd))[-2]) > #define FNTCHARCNT(fd) (((int *)(fd))[-3]) > #define FONT_EXTRA_WORDS 3 > > To keep things simple, move all these macro definitions to , > use four words instead of three, and initialize the fourth word in > newport_set_font() properly. > > Many thanks to Greg Kroah-Hartman , who > reviewed and improved this series! > > [1]: KASAN: global-out-of-bounds Read in fbcon_get_font > https://syzkaller.appspot.com/bug?id=08b8be45afea11888776f897895aef9ad1c3ecfd > > Peilin Ye (3): > fbdev, newport_con: Move FONT_EXTRA_WORDS macros into linux/font.h > Fonts: Support FONT_EXTRA_WORDS macros for built-in fonts > fbcon: Fix global-out-of-bounds read in fbcon_get_font() > > drivers/video/console/newport_con.c | 7 +------ > drivers/video/fbdev/core/fbcon.c | 12 ++++++++++++ > drivers/video/fbdev/core/fbcon.h | 7 ------- > drivers/video/fbdev/core/fbcon_rotate.c | 1 + > drivers/video/fbdev/core/tileblit.c | 1 + > include/linux/font.h | 13 +++++++++++++ > lib/fonts/font_10x18.c | 9 ++++----- > lib/fonts/font_6x10.c | 9 +++++---- > lib/fonts/font_6x11.c | 9 ++++----- > lib/fonts/font_7x14.c | 9 ++++----- > lib/fonts/font_8x16.c | 9 ++++----- > lib/fonts/font_8x8.c | 9 ++++----- > lib/fonts/font_acorn_8x8.c | 9 ++++++--- > lib/fonts/font_mini_4x6.c | 8 ++++---- > lib/fonts/font_pearl_8x8.c | 9 ++++----- > lib/fonts/font_sun12x22.c | 9 ++++----- > lib/fonts/font_sun8x16.c | 7 ++++--- > lib/fonts/font_ter16x32.c | 9 ++++----- > 18 files changed, 79 insertions(+), 67 deletions(-) Gotta love going backwards in arrays :) Nice work, whole series is: Reviewed-by: Greg Kroah-Hartman Daniel, can you take this through your tree? thanks, greg k-h