Virtualization - Aug 2013 - [PATCH v3 9/9] virtio: console: prevent use-after-free of port name in port unplug

Amit Shah <amit.shah at redhat.com> writes:
> Remove the debugfs path before freeing port->name, to prevent a possible
> use-after-free.
>
> Reported-by: Jason Wang <jasowang at redhat.com>
> Signed-off-by: Amit Shah <amit.shah at redhat.com>
Applied,
Rusty.
> ---
>  drivers/char/virtio_console.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index 4e380c1..e910bec 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -1574,9 +1574,8 @@ static void unplug_port(struct port *port)
>  	device_destroy(pdrvdata.class, port->dev->devt);
>  	cdev_del(port->cdev);
>  
> -	kfree(port->name);
> -
>  	debugfs_remove(port->debugfs_file);
> +	kfree(port->name);
>  
>  	/*
>  	 * Locks around here are not necessary - a port can't be
> -- 
> 1.8.1.4

On (Mon) 29 Jul 2013 [14:26:09], Rusty Russell wrote:
> Amit Shah <amit.shah at redhat.com> writes:
> > Remove the debugfs path before freeing port->name, to prevent a
possible
> > use-after-free.
> >
> > Reported-by: Jason Wang <jasowang at redhat.com>
> > Signed-off-by: Amit Shah <amit.shah at redhat.com>
> 
> Applied,
> Rusty.
Hey Rusty, I don't see this patch in your virtio-next branch:

https://git.kernel.org/cgit/linux/kernel/git/rusty/linux.git/log/?h=virtio-next

The others are present there.  Looks like this one got missed?

		Amit

Amit Shah <amit.shah at redhat.com> writes:
> On (Mon) 29 Jul 2013 [14:26:09], Rusty Russell wrote:
>> Amit Shah <amit.shah at redhat.com> writes:
>> > Remove the debugfs path before freeing port->name, to prevent a
possible
>> > use-after-free.
>> >
>> > Reported-by: Jason Wang <jasowang at redhat.com>
>> > Signed-off-by: Amit Shah <amit.shah at redhat.com>
>> 
>> Applied,
>> Rusty.
>
> Hey Rusty, I don't see this patch in your virtio-next branch:
>
>
https://git.kernel.org/cgit/linux/kernel/git/rusty/linux.git/log/?h=virtio-next
>
> The others are present there.  Looks like this one got missed?
Actually, it's in my pending-rebases branch, since it requireds things
from the fixes branch.  Once the fixes branch has gone to Linus (this
week) I will merge it into the virtio-next branch then apply this on
top.

It's generally considered bad form to merge into -next branches, but
it's allowed for cases like this.  But I want to make sure no changes to
fixes are required before I merge it, hence the delay for some exposure
in linux-next.

Cheers,
Rusty.