From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Jiri Slaby <jslaby@suse.com>, Dmitry Vyukov <dvyukov@google.com>,
linux-kernel@vger.kernel.org,
syzbot <syzbot+017265e8553724e514e8@syzkaller.appspotmail.com>
Subject: Re: [PATCH] vt: Reject zero-sized screen buffer size.
Date: Fri, 10 Jul 2020 12:55:00 +0200 [thread overview]
Message-ID: <20200710105500.GA1232395@kroah.com> (raw)
In-Reply-To: <20200710055329.3759-1-penguin-kernel@I-love.SAKURA.ne.jp>
On Fri, Jul 10, 2020 at 02:53:29PM +0900, Tetsuo Handa wrote:
> syzbot is reporting general protection fault in do_con_write() [1] caused
> by vc->vc_screenbuf == ZERO_SIZE_PTR caused by vc->vc_screenbuf_size == 0
> caused by vc->vc_cols == vc->vc_rows == vc->vc_size_row == 0 being passed
> to ioctl(FBIOPUT_VSCREENINFO) request on /dev/fb0 , for gotoxy(vc, 0, 0)
> from reset_terminal() from vc_init() from vc_allocate() on such console
> causes vc->vc_pos == 0x10000000e due to
> ((unsigned long) ZERO_SIZE_PTR) + -1U * 0 + (-1U << 1).
>
> I don't think that a console with 0 column and/or 0 row makes sense, and
> I think that we can reject such bogus arguments in fb_set_var() from
> ioctl(FBIOPUT_VSCREENINFO). Regardless, I think that it is safer to also
> check ZERO_SIZE_PTR when allocating vc->vc_screenbuf from vc_allocate()
> from con_install() from tty_init_dev() from tty_open().
>
> [1] https://syzkaller.appspot.com/bug?extid=017265e8553724e514e8
>
> Reported-by: syzbot <syzbot+017265e8553724e514e8@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> drivers/tty/vt/vt.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 48a8199f7845..8497e9206607 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -1126,7 +1126,7 @@ int vc_allocate(unsigned int currcons) /* return 0 on success */
> con_set_default_unimap(vc);
>
> vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_KERNEL);
> - if (!vc->vc_screenbuf)
> + if (ZERO_OR_NULL_PTR(vc->vc_screenbuf))
No, let's check this before we do kzalloc() please, that's just an odd
way of doing an allocation we shouldn't have had to do.
> goto err_free;
>
> /* If no drivers have overridden us and the user didn't pass a
> @@ -1212,7 +1212,7 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc,
> if (new_cols == vc->vc_cols && new_rows == vc->vc_rows)
> return 0;
>
> - if (new_screen_size > KMALLOC_MAX_SIZE)
> + if (new_screen_size > KMALLOC_MAX_SIZE || !new_screen_size)
> return -EINVAL;
> newscreen = kzalloc(new_screen_size, GFP_USER);
> if (!newscreen)
> @@ -3393,6 +3393,7 @@ static int __init con_init(void)
> INIT_WORK(&vc_cons[currcons].SAK_work, vc_SAK);
> tty_port_init(&vc->port);
> visual_init(vc, currcons, 1);
> + /* Assuming vc->vc_screenbuf_size is sane here, for this is __init code. */
Shouldn't we also check this here, or before we get here, too?
Just checking the values and rejecting that as a valid screen size
should be sufficient.
thanks,
greg k-h
next prev parent reply other threads:[~2020-07-10 10:55 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-10 5:53 [PATCH] vt: Reject zero-sized screen buffer size Tetsuo Handa
2020-07-10 5:56 ` fbconsole needs more parameter validations Tetsuo Handa
2020-07-10 10:56 ` Greg Kroah-Hartman
2020-07-11 6:16 ` Tetsuo Handa
2020-07-11 11:08 ` Tetsuo Handa
2020-07-12 11:10 ` [PATCH v3] vt: Reject zero-sized screen buffer size Tetsuo Handa
2020-07-12 11:10 ` [PATCH] fbdev: Detect integer underflow at "struct fbcon_ops"->clear_margins Tetsuo Handa
2020-07-14 7:22 ` Bartlomiej Zolnierkiewicz
2020-07-14 10:27 ` Tetsuo Handa
2020-07-14 13:37 ` Tetsuo Handa
2020-07-15 1:51 ` [PATCH v2] " Tetsuo Handa
2020-07-15 9:48 ` Dan Carpenter
2020-07-15 11:17 ` Tetsuo Handa
2020-07-15 14:02 ` Tetsuo Handa
2020-07-15 15:12 ` Dan Carpenter
2020-07-15 15:29 ` Tetsuo Handa
2020-07-16 10:00 ` Daniel Vetter
2020-07-16 11:27 ` Tetsuo Handa
2020-07-21 16:08 ` Greg Kroah-Hartman
2020-07-22 8:07 ` Daniel Vetter
2020-07-23 14:21 ` Greg Kroah-Hartman
2020-07-24 8:28 ` Bartlomiej Zolnierkiewicz
[not found] ` <c5bf6d5c-8d0a-8df5-2a11-38bf37a11d67@oracle.com>
2020-07-15 0:24 ` [PATCH] " Tetsuo Handa
2020-08-19 22:07 ` [PATCH v3] vt: Reject zero-sized screen buffer size Kees Cook
2020-07-10 10:55 ` Greg Kroah-Hartman [this message]
2020-07-10 11:31 ` [PATCH] " Tetsuo Handa
2020-07-10 11:36 ` Greg Kroah-Hartman
2020-07-10 14:34 ` [PATCH v2] " Tetsuo Handa
2020-07-20 15:40 ` Brooke Basile
2020-07-20 23:00 ` Tetsuo Handa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200710105500.GA1232395@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=dvyukov@google.com \
--cc=jslaby@suse.com \
--cc=linux-kernel@vger.kernel.org \
--cc=penguin-kernel@i-love.sakura.ne.jp \
--cc=syzbot+017265e8553724e514e8@syzkaller.appspotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox