[PATCH] hidraw : free list for all error in hidraw_open

Hi ,

In function hidraw_open (linux-3.0.3/drivers/hid/hidraw.c ) ,  struct
hidraw_list *list should be freed for all error conditions .
Following is patch enclosed for the same :

Signed-off-by: Amit Nagal <redacted>
---

diff -uprN linux-3.0.3/drivers/hid/hidraw.c.orig
linux-3.0.3/drivers/hid/hidraw.c

--- linux-3.0.3/drivers/hid/hidraw.c.orig       2011-09-01

13:23:19.000000000 -0400

+++ linux-3.0.3/drivers/hid/hidraw.c    2011-09-02 06:25:08.000000000 -0400

@@ -259,7 +259,6 @@ static int hidraw_open(struct inode *ino

        mutex_lock(&minors_lock);
        if (!hidraw_table[minor]) {
-               kfree(list);
                err = -ENODEV;
                goto out_unlock;
        }

@@ -285,6 +284,8 @@ static int hidraw_open(struct inode *ino
 out_unlock:
        mutex_unlock(&minors_lock);
 out:
+       if (err < 0)
+               kfree(list);
        return err;

 }

--

On Fri, Sep 2, 2011 at 10:59 AM, Amit Nagal [off-list ref] wrote:

Hi ,

In function hidraw_open (linux-3.0.3/drivers/hid/hidraw.c ) ,  struct
hidraw_list *list should be freed for all error conditions .

Due to tab to space conversion by gmail , i have enclosed patch as
attachment currently .

Regards
Amit

On Fri, 2 Sep 2011, Amit Nagal wrote:

quoted

In function hidraw_open (linux-3.0.3/drivers/hid/hidraw.c ) ,  struct
hidraw_list *list should be freed for all error conditions .

Due to tab to space conversion by gmail , i have enclosed patch as
attachment currently .

Applied. But please fix your mail client for further submissions. Thanks,

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 2 September 2011 06:29, Amit Nagal [off-list ref] wrote:

Hi ,

In function hidraw_open (linux-3.0.3/drivers/hid/hidraw.c ) ,  struct
hidraw_list *list should be freed for all error conditions .
Following is patch enclosed for the same :

Signed-off-by: Amit Nagal <redacted>

Another patch (3a22ebe9cc76acac2511b1d3979a35609924ce42 "HID: hidraw:
fix hidraw_disconnect()", pasted below) tried to fix the same problem,
but introduced a different problem where the device_destroy in
hidraw_disconnect would cause the hidraw to get freed, and then
hidraw_disconnect() would go and reference it then try and free it
again. I believe that patch should now be reverted, as with this patch
the device_destroy will free the list, and won't free the hidraw,
allowing hidraw_disconnect to clean it up itself. Does that sound
correct?

Cheers
James

commit 3a22ebe9cc76acac2511b1d3979a35609924ce42
Author: Stefan Achatz [off-list ref]
Date:   Sat Jan 29 02:17:30 2011 +0100

    HID: hidraw: fix hidraw_disconnect()

    hidraw_disconnect() first sets an entry in hidraw_table to NULL
    and calls device_destroy() afterwards. The thereby called
    hidraw_release() tries to read this already cleared value resulting
    in never removing any device from the list.
    This got fixed by changing the order of events.

    Signed-off-by: Stefan Achatz [off-list ref]
    Signed-off-by: Jiri Kosina [off-list ref]

diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index 468e87b..5516ea4 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c

@@ -428,12 +428,12 @@ void hidraw_disconnect(struct hid_device *hid)

        hidraw->exist = 0;

+ device_destroy(hidraw_class, MKDEV(hidraw_major, hidraw->minor));
+
        mutex_lock(&minors_lock);
        hidraw_table[hidraw->minor] = NULL;
        mutex_unlock(&minors_lock);

-   device_destroy(hidraw_class, MKDEV(hidraw_major, hidraw->minor));
-
        if (hidraw->open) {
                hid_hw_close(hid);
                wake_up_interruptible(&hidraw->wait);

-- 
James Hogan
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html