From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Eric Biggers <ebiggers@kernel.org>
Cc: Jiri Slaby <jslaby@suse.com>,
linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
syzkaller-bugs@googlegroups.com,
Eric Dumazet <edumazet@google.com>,
Nicolas Pitre <nico@fluxnic.net>
Subject: Re: [PATCH v2 1/2] vt: vt_ioctl: fix VT_DISALLOCATE freeing in-use virtual console
Date: Fri, 20 Mar 2020 07:57:59 +0100 [thread overview]
Message-ID: <20200320065759.GA307955@kroah.com> (raw)
In-Reply-To: <20200320051049.GA1315@sol.localdomain>
On Thu, Mar 19, 2020 at 10:10:49PM -0700, Eric Biggers wrote:
> On Thu, Mar 19, 2020 at 08:36:28AM +0100, Jiri Slaby wrote:
> > On 18. 03. 20, 23:38, Eric Biggers wrote:
> > > --- a/drivers/tty/vt/vt.c
> > > +++ b/drivers/tty/vt/vt.c
> > > @@ -1102,6 +1102,9 @@ int vc_allocate(unsigned int currcons) /* return 0 on success */
> > > tty_port_init(&vc->port);
> > > INIT_WORK(&vc_cons[currcons].SAK_work, vc_SAK);
> > >
> > > + /* if this wasn't the case, we'd have to implement port->ops.destruct */
> > > + BUILD_BUG_ON(offsetof(struct vc_data, port) != 0);
> > > +
> >
> > This is 3 lines, implementing destruct would be like 4-5 :)? Please
> > implement destruct instead.
> >
> > Otherwise looks good.
> >
>
> Actually implementing destruct would be 12 lines, see below. Remember there is
> no tty_port_operations defined yet so we'd have to define it just for destruct.
>
> Do you still prefer it?
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index ec34f1f5f3bb5..309a39197be0a 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -1075,6 +1075,17 @@ static void visual_deinit(struct vc_data *vc)
> module_put(vc->vc_sw->owner);
> }
>
> +static void vc_port_destruct(struct tty_port *port)
> +{
> + struct vc_data *vc = container_of(port, struct vc_data, port);
> +
> + kfree(vc);
> +}
> +
> +static const struct tty_port_operations vc_port_ops = {
> + .destruct = vc_port_destruct,
> +};
> +
> int vc_allocate(unsigned int currcons) /* return 0 on success */
> {
> struct vt_notifier_param param;
> @@ -1100,11 +1111,9 @@ int vc_allocate(unsigned int currcons) /* return 0 on success */
>
> vc_cons[currcons].d = vc;
> tty_port_init(&vc->port);
> + vc->port.ops = &vc_port_ops;
> INIT_WORK(&vc_cons[currcons].SAK_work, vc_SAK);
>
> - /* if this wasn't the case, we'd have to implement port->ops.destruct */
> - BUILD_BUG_ON(offsetof(struct vc_data, port) != 0);
> -
> visual_init(vc, currcons, 1);
>
> if (!*vc->vc_uni_pagedir_loc)
Yes, this is good to have, thanks for doing this.
greg k-h
next prev parent reply other threads:[~2020-03-20 6:58 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
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 [this message]
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=20200320065759.GA307955@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=ebiggers@kernel.org \
--cc=edumazet@google.com \
--cc=jslaby@suse.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).