Skip to contentSkip to conversation

input: psmouse: fix input_dev leak in lifebook driver

PATCH Applied Andres Salomon 2 participants 4 messages Jan 12, 2008 – Jan 16, 2008

Subsystems (2)
HID Input drivers
lore
Share & tools
lore

Andres Salomon

author
#0 lore raw

PATCH 1/1 input: psmouse: fix input_dev leak in lifebook driver

The lifebook driver may register a second input device, but it never unregisters it. This fixes that.

Signed-off-by Andres Salomon <[email protected]>
---
 drivers/input/mouse/lifebook.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)
drivers/input/mouse/lifebook.c +6 -1
index 9ec57d8..df81b0a 100644
@@ -225,8 +225,13 @@ static void lifebook_set_resolution(struct psmouse *psmouse, unsigned int resolu
 
 static void lifebook_disconnect(struct psmouse *psmouse)
 {
+	struct lifebook_data *priv = psmouse->private;
+
 	psmouse_reset(psmouse);
-	kfree(psmouse->private);
+	if (priv) {
+		input_unregister_device(priv->dev2);
+		kfree(priv);
+	}
 	psmouse->private = NULL;
 }
Signature · 1 line
1.5.3.5
maintainer Applied
#1 lore raw
Quoted 5 lines · Andres Salomon

The lifebook driver may register a second input device, but it never unregisters it. This fixes that.

Signed-off-by Andres Salomon <[email protected]>

Applied, thank you Andres.

Signature · 1 line
Dmitry
author
#2 lore raw
Quoted 9 lines · On Tue, 15 Jan 2008 17:04:01 -0500 Dmitry Torokhov <[email protected]>…
Quoted 5 lines · Andres Salomon

The lifebook driver may register a second input device, but it never unregisters it. This fixes that.

Signed-off-by Andres Salomon <[email protected]>

Applied, thank you Andres.

Hi Dmitry,

There's one additional patch, I don't know if you saw it; the subject was "check return value of input_register_device() in hil_ptr.c's init".

Also, I've found myself needing to provide my own device_attribute for the OLPC psmouse driver; I couldn't use PSMOUSE_DEFINE_ATTR(), as its set_helper callback calls psmouse_disable.

static DEVICE_ATTR(powered, S_IWUSR | S_IRUGO, hgpk_show_powered,

                hgpk_set_powered);

Unfortunately, in order to not be racy, hgpk_set_powered needs to deal with psmouse_mutex. Which method of dealing with this would you prefer? I could either make psmouse_mutex no longer static, provide psmouse_mutex locking functions (declared in psmouse.h), or put psmouse_mutex into 'struct psmouse'. Or, if you have any other ideas..

maintainer
#3 lore raw
Quoted 10 lines · Andres Salomon
Quoted 7 lines · On Tue, 15 Jan 2008 17:04:01 -0500 Dmitry Torokhov <[email protected]>…
Quoted 5 lines · Andres Salomon

The lifebook driver may register a second input device, but it never unregisters it. This fixes that.

Signed-off-by Andres Salomon <[email protected]>
Quoted 8 lines
Quoted 2 lines

Applied, thank you Andres.

Hi Dmitry,

There's one additional patch, I don't know if you saw it; the subject was "check return value of input_register_device() in hil_ptr.c's init".

I have seen it but postponed for later. Right now I am trying to get though 2.6.24 material.

Quoted 6 lines

Also, I've found myself needing to provide my own device_attribute for the OLPC psmouse driver; I couldn't use PSMOUSE_DEFINE_ATTR(), as its set_helper callback calls psmouse_disable.

static DEVICE_ATTR(powered, S_IWUSR | S_IRUGO, hgpk_show_powered,

Quoted 8 lines
                hgpk_set_powered);

Unfortunately, in order to not be racy, hgpk_set_powered needs to deal with psmouse_mutex. Which method of dealing with this would you prefer? I could either make psmouse_mutex no longer static, provide psmouse_mutex locking functions (declared in psmouse.h), or put psmouse_mutex into 'struct psmouse'. Or, if you have any other ideas..

Could you just send me the code? I tend to think better when I see it...

Thanks!

Signature · 1 line
Dmitry