From: Eric Biggers <ebiggers@kernel.org>
To: Jiri Slaby <jslaby@suse.cz>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com,
Eric Dumazet <edumazet@google.com>,
Nicolas Pitre <nico@fluxnic.net>,
Alan Cox <gnomes@lxorguk.ukuu.org.uk>
Subject: Re: [PATCH] vt: vt_ioctl: fix VT_DISALLOCATE freeing in-use virtual console
Date: Wed, 18 Mar 2020 15:27:04 -0700 [thread overview]
Message-ID: <20200318222704.GC2334@sol.localdomain> (raw)
In-Reply-To: <4ecf5517-f802-e1d3-5d0d-ba04fba58c87@suse.cz>
On Wed, Mar 18, 2020 at 02:15:00PM +0100, Jiri Slaby wrote:
> On 24. 02. 20, 9:19, Eric Biggers wrote:
> > On Mon, Feb 24, 2020 at 09:04:33AM +0100, Jiri Slaby wrote:
> >>> KASAN report:
> >>> BUG: KASAN: use-after-free in con_shutdown+0x76/0x80 drivers/tty/vt/vt.c:3278
> >>> Write of size 8 at addr ffff88806a4ec108 by task syz_vt/129
> >>>
> >>> CPU: 0 PID: 129 Comm: syz_vt Not tainted 5.6.0-rc2 #11
> >>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20191223_100556-anatol 04/01/2014
> >>> Call Trace:
> >>> [...]
> >>> con_shutdown+0x76/0x80 drivers/tty/vt/vt.c:3278
> >>> release_tty+0xa8/0x410 drivers/tty/tty_io.c:1514
> >>> tty_release_struct+0x34/0x50 drivers/tty/tty_io.c:1629
> >>> tty_release+0x984/0xed0 drivers/tty/tty_io.c:1789
> >>> [...]
> >>>
> >>> Allocated by task 129:
> >>> [...]
> >>> kzalloc include/linux/slab.h:669 [inline]
> >>> vc_allocate drivers/tty/vt/vt.c:1085 [inline]
> >>> vc_allocate+0x1ac/0x680 drivers/tty/vt/vt.c:1066
> >>> con_install+0x4d/0x3f0 drivers/tty/vt/vt.c:3229
> >>> tty_driver_install_tty drivers/tty/tty_io.c:1228 [inline]
> >>> tty_init_dev+0x94/0x350 drivers/tty/tty_io.c:1341
> >>> tty_open_by_driver drivers/tty/tty_io.c:1987 [inline]
> >>> tty_open+0x3ca/0xb30 drivers/tty/tty_io.c:2035
> >>> [...]
> >>>
> >>> Freed by task 130:
> >>> [...]
> >>> kfree+0xbf/0x1e0 mm/slab.c:3757
> >>> vt_disallocate drivers/tty/vt/vt_ioctl.c:300 [inline]
> >>> vt_ioctl+0x16dc/0x1e30 drivers/tty/vt/vt_ioctl.c:818
> >>> tty_ioctl+0x9db/0x11b0 drivers/tty/tty_io.c:2660
> >>
> >> That means the associated tty_port is destroyed while the tty layer
> >> still has a tty on the top of it. That is a BUG anyway.
>
> ...
>
> >> Locking tty_mutex here does not sound quite right. What about switching
> >> vc_data to proper refcounting based on tty_port? (Instead of doing
> >> tty_port_destroy and kfree in vt_disallocate*.)
> >>
> >
> > How would that work? We could make struct vc_data refcounted such that
> > VT_DISALLOCATE doesn't free it right away but rather it's freed in the next
> > con_shutdown(). But release_tty() still accesses tty->port afterwards, which is
> > part of the 'struct vc_data' that would have just been freed.
>
> Yes, but if it does the same as pty, i.e. throwing tty_port in
> ->cleanup, not ->shutdown, that would work, right?
>
> The initial requirement from tty_port is that it outlives tty. This was
> later lifted by pty to live at least till ->cleanup.
>
Yes, it looks like that will work. I didn't know about
tty_operations::cleanup(). I'll update the patch.
- Eric
next prev parent reply other threads:[~2020-03-18 22:27 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-03 20:15 KASAN: use-after-free Write in release_tty syzbot
2020-02-24 7:12 ` [PATCH] vt: vt_ioctl: fix VT_DISALLOCATE freeing in-use virtual console Eric Biggers
2020-02-24 8:04 ` Jiri Slaby
2020-02-24 8:19 ` Eric Biggers
2020-03-02 21:23 ` Eric Biggers
2020-03-18 10:06 ` Greg Kroah-Hartman
2020-03-18 10:10 ` Jiri Slaby
2020-03-18 13:15 ` Jiri Slaby
2020-03-18 22:27 ` Eric Biggers [this message]
2020-03-18 22:38 ` [PATCH v2 0/2] vt: fix some vt_ioctl races Eric Biggers
2020-03-18 22:38 ` [PATCH v2 1/2] vt: vt_ioctl: fix VT_DISALLOCATE freeing in-use virtual console Eric Biggers
2020-03-19 7:36 ` Jiri Slaby
2020-03-20 5:10 ` Eric Biggers
2020-03-20 6:57 ` Greg Kroah-Hartman
2020-03-18 22:38 ` [PATCH v2 2/2] vt: vt_ioctl: fix use-after-free in vt_in_use() Eric Biggers
2020-03-20 13:42 ` Jiri Slaby
2020-03-20 19:34 ` Eric Biggers
2020-03-22 3:43 ` [PATCH v3 0/2] vt: fix some vt_ioctl races Eric Biggers
2020-03-22 3:43 ` [PATCH v3 1/2] vt: vt_ioctl: fix VT_DISALLOCATE freeing in-use virtual console Eric Biggers
2020-03-27 10:28 ` Jiri Slaby
2020-03-22 3:43 ` [PATCH v3 2/2] vt: vt_ioctl: fix use-after-free in vt_in_use() Eric Biggers
2020-03-27 10:30 ` Jiri Slaby
2020-03-24 11:29 ` [PATCH v3 0/2] vt: fix some vt_ioctl races Greg Kroah-Hartman
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=20200318222704.GC2334@sol.localdomain \
--to=ebiggers@kernel.org \
--cc=edumazet@google.com \
--cc=gnomes@lxorguk.ukuu.org.uk \
--cc=gregkh@linuxfoundation.org \
--cc=jslaby@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=nico@fluxnic.net \
--cc=syzkaller-bugs@googlegroups.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.