[char-misc-next 1/3] mei: make me hw headers private to me hw.

[char-misc-next 1/3] mei: make me hw headers private to me hw.

[Tomas Winkler]

Remove includes of me-hw.h and me-hw-regs.h headers from
the mei generic code.
The mei layer should not depend on hw specific headers

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/amthif.c    | 1 -
 drivers/misc/mei/bus.c       | 1 -
 drivers/misc/mei/hbm.c       | 1 -
 drivers/misc/mei/hw-me.c     | 4 ++--
 drivers/misc/mei/interrupt.c | 1 -
 drivers/misc/mei/main.c      | 1 -
 drivers/misc/mei/mei_dev.h   | 1 -
 drivers/misc/mei/pci-me.c    | 3 ++-
 drivers/misc/mei/wd.c        | 1 -
 9 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/misc/mei/amthif.c b/drivers/misc/mei/amthif.c
index f05d54d..b75a7da 100644
--- a/drivers/misc/mei/amthif.c
+++ b/drivers/misc/mei/amthif.c
@@ -34,7 +34,6 @@
 
 #include "mei_dev.h"
 #include "hbm.h"
-#include "hw-me.h"
 #include "client.h"
 
 const uuid_le mei_amthif_guid  = UUID_LE(0x12f80028, 0xb4b7, 0x4b2d,
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 463ae51..ddc5ac9 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -26,7 +26,6 @@
 #include <linux/mei_cl_bus.h>
 
 #include "mei_dev.h"
-#include "hw-me.h"
 #include "client.h"
 
 #define to_mei_cl_driver(d) container_of(d, struct mei_cl_driver, driver)
diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c
index 9555791..5fa5ff9 100644
--- a/drivers/misc/mei/hbm.c
+++ b/drivers/misc/mei/hbm.c
@@ -21,7 +21,6 @@
 
 #include "mei_dev.h"
 #include "hbm.h"
-#include "hw-me.h"
 #include "client.h"
 
 static const char *mei_cl_conn_status_str(enum mei_cl_connect_status status)
diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index 7e769c5..d1994f6 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -20,10 +20,10 @@
 #include <linux/interrupt.h>
 
 #include "mei_dev.h"
-#include "hw-me.h"
-
 #include "hbm.h"
 
+#include "hw-me.h"
+#include "hw-me-regs.h"
 
 /**
  * mei_me_reg_read - Reads 32bit data from the mei device
diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index 2fbf0c0..e9c3fa8 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -26,7 +26,6 @@
 
 #include "mei_dev.h"
 #include "hbm.h"
-#include "hw-me.h"
 #include "client.h"
 
 
diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 49e3bb8..63ca2d1 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -40,7 +40,6 @@
 #include <linux/mei.h>
 
 #include "mei_dev.h"
-#include "hw-me.h"
 #include "client.h"
 
 /**
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 36640b9..2ef5593 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -24,7 +24,6 @@
 #include <linux/mei_cl_bus.h>
 
 #include "hw.h"
-#include "hw-me-regs.h"
 #include "hbm.h"
 
 /*
diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index 50d9cb5..19fa183 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -39,8 +39,9 @@
 #include <linux/mei.h>
 
 #include "mei_dev.h"
-#include "hw-me.h"
 #include "client.h"
+#include "hw-me-regs.h"
+#include "hw-me.h"
 
 /* mei_pci_tbl - PCI Device ID Table */
 static DEFINE_PCI_DEVICE_TABLE(mei_me_pci_tbl) = {
diff --git a/drivers/misc/mei/wd.c b/drivers/misc/mei/wd.c
index e084adf..ebf1cbc 100644
--- a/drivers/misc/mei/wd.c
+++ b/drivers/misc/mei/wd.c
@@ -25,7 +25,6 @@
 
 #include "mei_dev.h"
 #include "hbm.h"
-#include "hw-me.h"
 #include "client.h"
 
 static const u8 mei_start_wd_params[] = { 0x02, 0x12, 0x13, 0x10 };
-- 
1.8.5.3

[char-misc-next 2/3] mei: drop pr_fmt macros

[Tomas Winkler]

we are using dev_ logging facility so we don't need pr_fmt macros

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Reviewed-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/misc/mei/main.c   | 3 ---
 drivers/misc/mei/pci-me.c | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 63ca2d1..4e069b8 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -13,9 +13,6 @@
  * more details.
  *
  */
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/kernel.h>
diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index 19fa183..135ea06 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -13,9 +13,6 @@
  * more details.
  *
  */
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/kernel.h>
-- 
1.8.5.3

[char-misc-next 3/3] mei: report consistently copy_from/to_user failures

[Tomas Winkler]

From: Alexander Usyskin <alexander.usyskin@intel.com>

Consistently display error on possible copy_from/to_user failures
and replace dev_dbg with dev_err

Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/amthif.c | 4 +++-
 drivers/misc/mei/main.c   | 3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/mei/amthif.c b/drivers/misc/mei/amthif.c
index b75a7da..2e7d914 100644
--- a/drivers/misc/mei/amthif.c
+++ b/drivers/misc/mei/amthif.c
@@ -244,8 +244,10 @@ int mei_amthif_read(struct mei_device *dev, struct file *file,
 	 * the buf_idx may point beyond */
 	length = min_t(size_t, length, (cb->buf_idx - *offset));
 
-	if (copy_to_user(ubuf, cb->response_buffer.data + *offset, length))
+	if (copy_to_user(ubuf, cb->response_buffer.data + *offset, length)) {
+		dev_err(&dev->pdev->dev, "failed to copy data to userland\n");
 		rets = -EFAULT;
+	}
 	else {
 		rets = length;
 		if ((*offset + length) < cb->buf_idx) {
diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 4e069b8..819aa8b 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -280,6 +280,7 @@ copy_buffer:
 	length = min_t(size_t, length, cb->buf_idx - *offset);
 
 	if (copy_to_user(ubuf, cb->response_buffer.data + *offset, length)) {
+		dev_err(&dev->pdev->dev, "failed to copy data to userland\n");
 		rets = -EFAULT;
 		goto free;
 	}
@@ -579,7 +580,7 @@ static long mei_ioctl(struct file *file, unsigned int cmd, unsigned long data)
 	dev_dbg(&dev->pdev->dev, "copy connect data to user\n");
 	if (copy_to_user((char __user *)data, connect_data,
 				sizeof(struct mei_connect_client_data))) {
-		dev_dbg(&dev->pdev->dev, "failed to copy data to userland\n");
+		dev_err(&dev->pdev->dev, "failed to copy data to userland\n");
 		rets = -EFAULT;
 		goto out;
 	}
-- 
1.8.5.3

Re: [char-misc-next 3/3] mei: report consistently copy_from/to_user failures

[One Thousand Gnomes]

On Tue, 11 Mar 2014 14:49:25 +0200
Tomas Winkler <tomas.winkler@intel.com> wrote:

> From: Alexander Usyskin <alexander.usyskin@intel.com>
> 
> Consistently display error on possible copy_from/to_user failures
> and replace dev_dbg with dev_err

NAK

This allows users to flood the logs by deliberately causing lots of these
failed to copy data error messages, and if they can flood the logs they
can hide other stuff.

The -EFAULT cases should be consistent - but consistently *not* logged.

Alan

RE: [char-misc-next 3/3] mei: report consistently copy_from/to_user failures

[Winkler, Tomas]

> On Tue, 11 Mar 2014 14:49:25 +0200
> Tomas Winkler <tomas.winkler@intel.com> wrote:
> 
> > From: Alexander Usyskin <alexander.usyskin@intel.com>
> >
> > Consistently display error on possible copy_from/to_user failures
> > and replace dev_dbg with dev_err
> 
> NAK
> 
> This allows users to flood the logs by deliberately causing lots of these
> failed to copy data error messages, and if they can flood the logs they
> can hide other stuff.
> 
> The -EFAULT cases should be consistent - but consistently *not* logged.

Yep, that's really looks like a possible exploit, please drop this patch.
Tomas