Re: [PATCH 03/10] virtio: console: clean up port data immediately at time of unplug

[Jason Wang <jasowang@redhat.com>]

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