[char-misc-next 0/3] mei reset cleanups

[char-misc-next 0/3] mei reset cleanups

[Tomas Winkler]

some more reset cleanups, less urgent then 
the series that went for 3.11

And fix for one non likely to happen security issue


Tomas Winkler (3):
  mei: wake also writers on reset
  mei: don't get stack in select during reset
  mei: bus: do not overflow the device name buffer

 drivers/misc/mei/amthif.c | 14 +++++++++++---
 drivers/misc/mei/bus.c    |  4 ++--
 drivers/misc/mei/client.c | 15 ++++++++-------
 drivers/misc/mei/client.h |  9 ++++++++-
 drivers/misc/mei/init.c   | 13 ++++++++-----
 drivers/misc/mei/main.c   | 22 +++++++++++++++-------
 6 files changed, 52 insertions(+), 25 deletions(-)

-- 
1.8.1.2

[char-misc-next 1/3] mei: wake also writers on reset

[Tomas Winkler]

wake writers otherwise might have processes waiting
endlessly on wait_tx during reset

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/client.c | 10 +++++++---
 drivers/misc/mei/client.h |  2 +-
 drivers/misc/mei/init.c   |  2 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 21d3f5a..af1e602 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -892,18 +892,22 @@ void mei_cl_all_disconnect(struct mei_device *dev)
 
 
 /**
- * mei_cl_all_read_wakeup  - wake up all readings so they can be interrupted
+ * mei_cl_all_wakeup  - wake up all readers and writers they can be interrupted
  *
  * @dev  - mei device
  */
-void mei_cl_all_read_wakeup(struct mei_device *dev)
+void mei_cl_all_wakeup(struct mei_device *dev)
 {
 	struct mei_cl *cl, *next;
 	list_for_each_entry_safe(cl, next, &dev->file_list, link) {
 		if (waitqueue_active(&cl->rx_wait)) {
-			dev_dbg(&dev->pdev->dev, "Waking up client!\n");
+			dev_dbg(&dev->pdev->dev, "Waking up reading client!\n");
 			wake_up_interruptible(&cl->rx_wait);
 		}
+		if (waitqueue_active(&cl->tx_wait)) {
+			dev_dbg(&dev->pdev->dev, "Waking up writing client!\n");
+			wake_up_interruptible(&cl->tx_wait);
+		}
 	}
 }
 
diff --git a/drivers/misc/mei/client.h b/drivers/misc/mei/client.h
index 26b157d..9bae4c7 100644
--- a/drivers/misc/mei/client.h
+++ b/drivers/misc/mei/client.h
@@ -99,7 +99,7 @@ void mei_host_client_init(struct work_struct *work);
 
 
 void mei_cl_all_disconnect(struct mei_device *dev);
-void mei_cl_all_read_wakeup(struct mei_device *dev);
+void mei_cl_all_wakeup(struct mei_device *dev);
 void mei_cl_all_write_clear(struct mei_device *dev);
 
 #endif /* _MEI_CLIENT_H_ */
diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c
index e6f16f8..4bb7f3c 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -197,7 +197,7 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled)
 	mei_hbm_start_req(dev);
 
 	/* wake up all readings so they can be interrupted */
-	mei_cl_all_read_wakeup(dev);
+	mei_cl_all_wakeup(dev);
 
 	/* remove all waiting requests */
 	mei_cl_all_write_clear(dev);
-- 
1.8.1.2

[char-misc-next 2/3] mei: don't get stack in select during reset

[Tomas Winkler]

Clear pending connection after hw reset but before hw start
and wake up the waiting task in poll. Signal POLLERR in select
when device went through reset.

Add wrapper mei_cl_is_connected for checking if
the device and client are connected.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/amthif.c | 14 +++++++++++---
 drivers/misc/mei/client.c |  5 +----
 drivers/misc/mei/client.h |  7 +++++++
 drivers/misc/mei/init.c   | 13 ++++++++-----
 drivers/misc/mei/main.c   | 22 +++++++++++++++-------
 5 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/drivers/misc/mei/amthif.c b/drivers/misc/mei/amthif.c
index 749452f..d0fdc13 100644
--- a/drivers/misc/mei/amthif.c
+++ b/drivers/misc/mei/amthif.c
@@ -418,15 +418,23 @@ unsigned int mei_amthif_poll(struct mei_device *dev,
 		struct file *file, poll_table *wait)
 {
 	unsigned int mask = 0;
-	mutex_unlock(&dev->device_lock);
+
 	poll_wait(file, &dev->iamthif_cl.wait, wait);
+
 	mutex_lock(&dev->device_lock);
-	if (dev->iamthif_state == MEI_IAMTHIF_READ_COMPLETE &&
-		dev->iamthif_file_object == file) {
+	if (!mei_cl_is_connected(&dev->iamthif_cl)) {
+
+		mask = POLLERR;
+
+	} else if (dev->iamthif_state == MEI_IAMTHIF_READ_COMPLETE &&
+		   dev->iamthif_file_object == file) {
+
 		mask |= (POLLIN | POLLRDNORM);
 		dev_dbg(&dev->pdev->dev, "run next amthif cb\n");
 		mei_amthif_run_next_cmd(dev);
 	}
+	mutex_unlock(&dev->device_lock);
+
 	return mask;
 }
 
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index af1e602..e0684b4 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -635,10 +635,7 @@ int mei_cl_read_start(struct mei_cl *cl, size_t length)
 
 	dev = cl->dev;
 
-	if (cl->state != MEI_FILE_CONNECTED)
-		return -ENODEV;
-
-	if (dev->dev_state != MEI_DEV_ENABLED)
+	if (!mei_cl_is_connected(cl))
 		return -ENODEV;
 
 	if (cl->read_cb) {
diff --git a/drivers/misc/mei/client.h b/drivers/misc/mei/client.h
index 9bae4c7..9eb031e 100644
--- a/drivers/misc/mei/client.h
+++ b/drivers/misc/mei/client.h
@@ -84,6 +84,13 @@ int mei_cl_flow_ctrl_reduce(struct mei_cl *cl);
 /*
  *  MEI input output function prototype
  */
+static inline bool mei_cl_is_connected(struct mei_cl *cl)
+{
+	return (cl->dev &&
+		cl->dev->dev_state == MEI_DEV_ENABLED &&
+		cl->state == MEI_FILE_CONNECTED);
+}
+
 bool mei_cl_is_other_connecting(struct mei_cl *cl);
 int mei_cl_disconnect(struct mei_cl *cl);
 int mei_cl_connect(struct mei_cl *cl, struct file *file);
diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c
index 4bb7f3c..e38e3d89 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -150,12 +150,20 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled)
 
 	if (dev->dev_state != MEI_DEV_INITIALIZING &&
 	    dev->dev_state != MEI_DEV_POWER_UP) {
+
 		if (dev->dev_state != MEI_DEV_DISABLED &&
 		    dev->dev_state != MEI_DEV_POWER_DOWN)
 			dev->dev_state = MEI_DEV_RESETTING;
 
+
+		/* remove all waiting requests */
+		mei_cl_all_write_clear(dev);
+
 		mei_cl_all_disconnect(dev);
 
+		/* wake up all readings so they can be interrupted */
+		mei_cl_all_wakeup(dev);
+
 		/* remove entry if already in list */
 		dev_dbg(&dev->pdev->dev, "remove iamthif and wd from the file list.\n");
 		mei_cl_unlink(&dev->wd_cl);
@@ -196,11 +204,6 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled)
 
 	mei_hbm_start_req(dev);
 
-	/* wake up all readings so they can be interrupted */
-	mei_cl_all_wakeup(dev);
-
-	/* remove all waiting requests */
-	mei_cl_all_write_clear(dev);
 }
 EXPORT_SYMBOL_GPL(mei_reset);
 
diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 5e11b5b..173ff09 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -625,24 +625,32 @@ static unsigned int mei_poll(struct file *file, poll_table *wait)
 	unsigned int mask = 0;
 
 	if (WARN_ON(!cl || !cl->dev))
-		return mask;
+		return POLLERR;
 
 	dev = cl->dev;
 
 	mutex_lock(&dev->device_lock);
 
-	if (dev->dev_state != MEI_DEV_ENABLED)
-		goto out;
-
-
-	if (cl == &dev->iamthif_cl) {
-		mask = mei_amthif_poll(dev, file, wait);
+	if (!mei_cl_is_connected(cl)) {
+		mask = POLLERR;
 		goto out;
 	}
 
 	mutex_unlock(&dev->device_lock);
+
+
+	if (cl == &dev->iamthif_cl)
+		return mei_amthif_poll(dev, file, wait);
+
 	poll_wait(file, &cl->tx_wait, wait);
+
 	mutex_lock(&dev->device_lock);
+
+	if (!mei_cl_is_connected(cl)) {
+		mask = POLLERR;
+		goto out;
+	}
+
 	if (MEI_WRITE_COMPLETE == cl->writing_state)
 		mask |= (POLLIN | POLLRDNORM);
 
-- 
1.8.1.2

Re: [char-misc-next 2/3] mei: don't get stack in select during reset

[Greg KH]

On Wed, Jul 24, 2013 at 04:22:58PM +0300, Tomas Winkler wrote:
> Clear pending connection after hw reset but before hw start
> and wake up the waiting task in poll. Signal POLLERR in select
> when device went through reset.
> 
> Add wrapper mei_cl_is_connected for checking if
> the device and client are connected.
> 
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
>  drivers/misc/mei/amthif.c | 14 +++++++++++---
>  drivers/misc/mei/client.c |  5 +----
>  drivers/misc/mei/client.h |  7 +++++++
>  drivers/misc/mei/init.c   | 13 ++++++++-----
>  drivers/misc/mei/main.c   | 22 +++++++++++++++-------
>  5 files changed, 42 insertions(+), 19 deletions(-)

This patch fails to apply, can you refresh it and resend?

thanks,

greg k-h

[char-misc-next 3/3] mei: bus: do not overflow the device name buffer

[Tomas Winkler]

1. use strncmp for comparsion strncpy was used for copying
which may omit the final %NUL terminator
2. id->name is statically defined so we can use sizeof

Acked-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/bus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 9ecd49a..a150a42 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -47,7 +47,7 @@ static int mei_cl_device_match(struct device *dev, struct device_driver *drv)
 	id = driver->id_table;
 
 	while (id->name[0]) {
-		if (!strcmp(dev_name(dev), id->name))
+		if (!strncmp(dev_name(dev), id->name, sizeof(id->name)))
 			return 1;
 
 		id++;
@@ -71,7 +71,7 @@ static int mei_cl_device_probe(struct device *dev)
 
 	dev_dbg(dev, "Device probe\n");
 
-	strncpy(id.name, dev_name(dev), MEI_CL_NAME_SIZE);
+	strncpy(id.name, dev_name(dev), sizeof(id.name));
 
 	return driver->probe(device, &id);
 }
-- 
1.8.1.2