[patch 0/2] two hiddev fixes [-rc regressions]

[patch 0/2] two hiddev fixes [-rc regressions]

[Johannes Weiner]

Hi,

here are two patches for hiddev bugs that were introduced past .28.  I
think patch #2 fixes the 5th top bug on kerneloops.org for this
release cycle.  More details in the changelogs.

Well, no kerneloops link, the site just keeps time-outing on me...
It's the finish_wait() entry with 23 reports.

	Hannes

[patch 1/2] hiddev: fix incorrect free

[Johannes Weiner]

[-- Attachment #1: hiddev-fix-incorrect-free.patch --]
[-- Type: text/plain, Size: 756 bytes --]

If hiddev_open() fails, it wrongly frees the shared hiddev structure
kept in hiddev_table instead of the hiddev_list structure allocated
for the opened file descriptor.  Existing references to this structure
will then accessed free memory.

This was introduced by 079034073 "HID: hiddev cleanup -- handle all
error conditions properly".

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Cc: Oliver Neukum <oliver@neukum.name>
---
 drivers/hid/usbhid/hiddev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -306,7 +306,7 @@ static int hiddev_open(struct inode *ino
 	return 0;
 bail:
 	file->private_data = NULL;
-	kfree(list->hiddev);
+	kfree(list);
 	return res;
 }
 

[patch 2/2] hiddev: fix waitqueue usage

[Johannes Weiner]

[-- Attachment #1: hiddev-fix-waitqueue-usage.patch --]
[-- Type: text/plain, Size: 1173 bytes --]

DECLARE_WAITQUEUE doesn't initialize the wait descriptor's task_list
to 'empty' but to zero.

prepare_to_wait() will not enqueue the descriptor to the waitqueue and
finish_wait() will do list_del_init() on a list head that contains
NULL pointers, which oopses.

This was introduced by 079034073 "HID: hiddev cleanup -- handle all
error conditions properly".

The prior code used an unconditional add_to_waitqueue() which didn't
care about the wait descriptor's list head and enqueued the thing
unconditionally.

The new code uses prepare_to_wait() which DOES check the prior list
state, so use DEFINE_WAIT instead.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Cc: Oliver Neukum <oliver@neukum.name>
---
 drivers/hid/usbhid/hiddev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -323,7 +323,7 @@ static ssize_t hiddev_write(struct file 
  */
 static ssize_t hiddev_read(struct file * file, char __user * buffer, size_t count, loff_t *ppos)
 {
-	DECLARE_WAITQUEUE(wait, current);
+	DEFINE_WAIT(wait);
 	struct hiddev_list *list = file->private_data;
 	int event_size;
 	int retval;

Re: [patch 0/2] two hiddev fixes [-rc regressions]

[Jiri Kosina]

On Mon, 9 Mar 2009, Johannes Weiner wrote:

> here are two patches for hiddev bugs that were introduced past .28.  I
> think patch #2 fixes the 5th top bug on kerneloops.org for this
> release cycle.  More details in the changelogs.
> 
> Well, no kerneloops link, the site just keeps time-outing on me...
> It's the finish_wait() entry with 23 reports.

I have applied both patches, thanks a lot for fixing this, Hannes.

-- 
Jiri Kosina
SUSE Labs