On (Fri) 19 Jul 2013 [13:11:54], Jason Wang wrote:
> On 07/19/2013 01:02 PM, Amit Shah wrote:
> > On (Fri) 19 Jul 2013 [11:21:47], Jason Wang wrote:
> >> On 07/19/2013 04:16 AM, Amit Shah wrote:
> >
> >>> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> >>> index b04ec95..6bf0df3 100644
> >>> --- a/drivers/char/virtio_console.c
> >>> +++ b/drivers/char/virtio_console.c
> >>> @@ -1501,14 +1501,6 @@ static void remove_port(struct kref *kref)
> >>>
> >>> port = container_of(kref, struct port, kref);
> >>>
> >>> - sysfs_remove_group(&port->dev->kobj, &port_attribute_group);
> >>> - device_destroy(pdrvdata.class, port->dev->devt);
> >>> - cdev_del(port->cdev);
> >>> -
> >>> - kfree(port->name);
> >>> -
> >>> - debugfs_remove(port->debugfs_file);
> >>> -
> >>> kfree(port);
> >>> }
> >>>
> >>> @@ -1566,6 +1558,14 @@ static void unplug_port(struct port *port)
> >>> */
> >>> port->portdev = NULL;
> >>>
> >>> + sysfs_remove_group(&port->dev->kobj, &port_attribute_group);
> >>> + device_destroy(pdrvdata.class, port->dev->devt);
> >>> + cdev_del(port->cdev);
> >>> +
> >>> + kfree(port->name);
> >>> +
> >>> + debugfs_remove(port->debugfs_file);
> >>> +
> >>> /*
> >>> * Locks around here are not necessary - a port can't be
> >>> * opened after we removed the port struct from ports_list
> >> Should we remove debugfs file before kfree()? Otherwise looks like a
> >> use-after-free if user access debugfs after kfree().
> > It is removed before kfree() -- kfree() is called in remove_port(),
> > which is called when all the references are dropped. (Did you confuse
> > kfree(port->name) with kfree(port)?)
>
> Nope. Looks like port->name were accessed in debugfs_read()?
Ah, got it. Since this bug existed before this patch as well, I'll
post an additional patch on top of this.
Thanks!
Amit
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
