Thread (28 messages) 28 messages, 6 authors, 2017-04-01

Re: [PATCH 6/8] Input: psmouse - add support for SMBus companions

flat view

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2017-03-10 18:16:28
Also in: lkml

On Fri, Mar 10, 2017 at 06:55:09PM +0100, Benjamin Tissoires wrote:
quoted hunk ↗ jump to hunk
On Mar 09 2017 or thereabouts, Dmitry Torokhov wrote:
quoted
From: Benjamin Tissoires <redacted>

This provides glue between PS/2 devices that enumerate the RMI4 devices
and Elan touchpads to the RMI4 (or Elan) SMBus driver.

The SMBus devices keep their PS/2 connection alive. If the initialization
process goes too far (psmouse_activate called), the device disconnects
from the I2C bus and stays on the PS/2 bus, that is why we explicitly
disable PS/2 device reporting (by calling psmouse_deactivate) before
trying to register SMBus companion device.

The HID over I2C devices are enumerated through the ACPI DSDT, and
their PS/2 device also exports the InterTouch bit in the extended
capability 0x0C. However, the firmware keeps its I2C connection open
even after going further in the PS/2 initialization. We don't need
to take extra precautions with those device, especially because they
block their PS/2 communication when HID over I2C is used.

Signed-off-by: Benjamin Tissoires <redacted>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
Hi Dmitry,

There is an issue with this patch. The platform_data provided by the
caller of psmouse_smbus_init() may be dereference, leading to a dangling
pointer on the stack in rmi4.

There is no guarantees rmi-smbus will get probed directly and even if it
does, a later rmmod/modprobe might happen and won't have access to the
platform data.

See below for a patch that solves this. Feel free to squash it, change it
and remove the terrible commit message.

From a86f766de9c544a8d61da520719287c68a5f1bea Mon Sep 17 00:00:00 2001
From: Benjamin Tissoires <redacted>
Date: Fri, 10 Mar 2017 18:31:01 +0100
Subject: [PATCH] Input: smbus companion - store the platform data for later
 use

The platform data should be available as long as the i2c_device exists.
In the current implementation, the platform data is allocated on the
stack, which gives a dangling pointer to the i2c_client once
psmouse_smbus_init() ends.

Duplicate the provided platform data in psmouse_smbus_init() so that
the allocation/free of pdata is handled by psmouse-smbus.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/input/mouse/psmouse-smbus.c | 18 ++++++++++++++++--
 drivers/input/mouse/psmouse.h       |  2 +-
 drivers/input/mouse/synaptics.c     |  5 ++---
 3 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/input/mouse/psmouse-smbus.c b/drivers/input/mouse/psmouse-smbus.c
index 5bda551..6f603ec 100644
--- a/drivers/input/mouse/psmouse-smbus.c
+++ b/drivers/input/mouse/psmouse-smbus.c
@@ -20,6 +20,7 @@
 struct psmouse_smbus_dev {
 	struct psmouse *psmouse;
 	struct i2c_client *client;
+	void *pdata;
 	struct list_head node;
 	unsigned short addr;
 	bool dead;
@@ -166,6 +167,7 @@ static void psmouse_smbus_disconnect(struct psmouse *psmouse)
 		psmouse_smbus_schedule_remove(smbdev->client);
 	}
 
+	kfree(smbdev->pdata);
We can 't do it here if we fired off a work to unregister i2c_client, as
it may still be referencing it. I guess we can do it in notifier code.
I'll update the patch.

Thanks.

-- 
Dmitry
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help