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()? > > Thanks, > > Amit -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html