Re: [patch] hiddev: fix incorrect hiddev freeing

[Johannes Weiner <hannes@cmpxchg.org>]

On Mon, Mar 09, 2009 at 03:31:51AM +0100, Johannes Weiner wrote:
> When hiddev_open() fails for whatever reason, free the just allocated
> hiddev_list structure shared hiddev potentially still in use.
> 
> The hiddev is freed in device disconnect/last close of the device file
> and must not be freed while there are possibly existing references to
> it.
> 
> This is probably responsible for these
> 
>   http://kerneloops.org/oops.php?number=221185
>   http://kerneloops.org/oops.php?number=220365
> 
> where a reader sleeps on the waitqueue, the device gets disconnected
> (exist -> 0) another user tries to open it, fails on the exist check
> and frees the hiddev from the table.  The finish_wait() in the reader
> will then dereference the hiddev to get to the waitqueue and oopses.
> 
> This was introduced by commit 079034073faf974973baa0256b029451f6e768ad
> "HID: hiddev cleanup -- handle all error conditions properly".
> 
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Oliver Neukum <oliver@neukum.name>
> ---
> 
> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> index 4940e4d..00ea1ed 100644
> --- a/drivers/hid/usbhid/hiddev.c
> +++ b/drivers/hid/usbhid/hiddev.c
> @@ -306,7 +306,7 @@ static int hiddev_open(struct inode *inode, struct file *file)
>  	return 0;
>  bail:
>  	file->private_data = NULL;
> -	kfree(list->hiddev);
> +	kfree(list);
>  	return res;

This isn't responsible for the above quoted oopsen but I think I found
the real issue.  Resend coming soon.

Dear stable team, I mixed up the commit date with the authoring date.
The bugs were introduced after .28, so there is no need to backport
anything unless my fixes fail to get into .29.  So please ignore for
now.

	Hannes