[char-misc-next 0/5] mei driver small fixes and cleanups

[char-misc-next 0/5] mei driver small fixes and cleanups

[Tomas Winkler]

Alexander Usyskin (1):
  mei: mei_write correct checks for copy_from_user

Tomas Winkler (4):
  mei: mei_cl_link protect open_handle_count from overflow
  mei: make sure that me_clients_map big enough before copying
  mei: fix format compilation warrning on 32 bit architecture
  mei: revamp read and write length checks

 drivers/misc/mei/client.c |  6 ++++++
 drivers/misc/mei/hbm.c    |  7 +++++--
 drivers/misc/mei/main.c   | 23 +++++++++++++++++++----
 3 files changed, 30 insertions(+), 6 deletions(-)

-- 
1.8.3.1

[char-misc-next 1/5] mei: mei_cl_link protect open_handle_count from overflow

[Tomas Winkler]

mei_cl_link is called both from mei_open and also from
in-kernel drivers so we need to protect open_handle_count
from overflow

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/client.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index e0684b4..a82b443 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -287,6 +287,12 @@ int mei_cl_link(struct mei_cl *cl, int id)
 		return -ENOENT;
 	}
 
+	if (dev->open_handle_count >= MEI_MAX_OPEN_HANDLE_COUNT) {
+		dev_err(&dev->pdev->dev, "open_handle_count exceded %d",
+			MEI_MAX_OPEN_HANDLE_COUNT);
+		return -ENOENT;
+	}
+
 	dev->open_handle_count++;
 
 	cl->host_client_id = id;
-- 
1.8.3.1

[char-misc-next 2/5] mei: make sure that me_clients_map big enough before copying

[Tomas Winkler]

To make static analyzers happy validated that
sizeof me_clients_map  is larger than sizeof valid_addresses from the
enumeration response before memcpy
We can use BUILD_ON macro as both arrays are defined statically

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/hbm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c
index 6127ab6..95d4dab 100644
--- a/drivers/misc/mei/hbm.c
+++ b/drivers/misc/mei/hbm.c
@@ -673,7 +673,10 @@ void mei_hbm_dispatch(struct mei_device *dev, struct mei_msg_hdr *hdr)
 
 	case HOST_ENUM_RES_CMD:
 		enum_res = (struct hbm_host_enum_response *) mei_msg;
-		memcpy(dev->me_clients_map, enum_res->valid_addresses, 32);
+		BUILD_BUG_ON(sizeof(dev->me_clients_map)
+				< sizeof(enum_res->valid_addresses));
+		memcpy(dev->me_clients_map, enum_res->valid_addresses,
+			sizeof(enum_res->valid_addresses));
 		if (dev->dev_state == MEI_DEV_INIT_CLIENTS &&
 		    dev->hbm_state == MEI_HBM_ENUM_CLIENTS) {
 				dev->init_clients_timer = 0;
-- 
1.8.3.1

[char-misc-next 3/5] mei: mei_write correct checks for copy_from_user

[Tomas Winkler]

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

1. return -EFUALT when copy_from_user fails
2. display error message on failure in error level

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

diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 173ff09..5ff810b 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -404,8 +404,11 @@ static ssize_t mei_write(struct file *file, const char __user *ubuf,
 		goto out;
 
 	rets = copy_from_user(write_cb->request_buffer.data, ubuf, length);
-	if (rets)
+	if (rets) {
+		dev_err(&dev->pdev->dev, "failed to copy data from userland\n");
+		rets = -EFAULT;
 		goto out;
+	}
 
 	if (cl == &dev->iamthif_cl) {
 		rets = mei_amthif_write(dev, write_cb);
@@ -567,7 +570,7 @@ static long mei_ioctl(struct file *file, unsigned int cmd, unsigned long data)
 	dev_dbg(&dev->pdev->dev, "copy connect data from user\n");
 	if (copy_from_user(connect_data, (char __user *)data,
 				sizeof(struct mei_connect_client_data))) {
-		dev_dbg(&dev->pdev->dev, "failed to copy data from userland\n");
+		dev_err(&dev->pdev->dev, "failed to copy data from userland\n");
 		rets = -EFAULT;
 		goto out;
 	}
-- 
1.8.3.1

[char-misc-next 4/5] mei: fix format compilation warrning on 32 bit architecture

[Tomas Winkler]

hbm.c: In function mei_hbm_me_cl_allocate:
hbm.c:52:212: warning: format %zd expects argument of type signed size_t but argument 4 has type long unsigned

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/hbm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c
index 95d4dab..f706fe8 100644
--- a/drivers/misc/mei/hbm.c
+++ b/drivers/misc/mei/hbm.c
@@ -45,7 +45,7 @@ static void mei_hbm_me_cl_allocate(struct mei_device *dev)
 	kfree(dev->me_clients);
 	dev->me_clients = NULL;
 
-	dev_dbg(&dev->pdev->dev, "memory allocation for ME clients size=%zd.\n",
+	dev_dbg(&dev->pdev->dev, "memory allocation for ME clients size=%ld.\n",
 		dev->me_clients_num * sizeof(struct mei_me_client));
 	/* allocate storage for ME clients representation */
 	clients = kcalloc(dev->me_clients_num,
-- 
1.8.3.1

[char-misc-next 5/5] mei: revamp read and write length checks

[Tomas Winkler]

1. Return zero on zero length read and writes
2. For a too large write return -EFBIG as defined in man write(2)
EFBIG  An attempt was made to write a file that
        exceeds the implementation-defined maximum
        file size or the process's file size limit,
        or to  write  at  a  position  past  the  maximum
        allowed offset.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/main.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 5ff810b..7404584 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -203,12 +203,18 @@ static ssize_t mei_read(struct file *file, char __user *ubuf,
 
 	dev = cl->dev;
 
+
 	mutex_lock(&dev->device_lock);
 	if (dev->dev_state != MEI_DEV_ENABLED) {
 		rets = -ENODEV;
 		goto out;
 	}
 
+	if (length == 0) {
+		rets = 0;
+		goto out;
+	}
+
 	if (cl == &dev->iamthif_cl) {
 		rets = mei_amthif_read(dev, file, ubuf, length, offset);
 		goto out;
@@ -350,8 +356,14 @@ static ssize_t mei_write(struct file *file, const char __user *ubuf,
 		rets = -ENODEV;
 		goto out;
 	}
-	if (length > dev->me_clients[id].props.max_msg_length || length <= 0) {
-		rets = -EMSGSIZE;
+
+	if (length == 0) {
+		rets = 0;
+		goto out;
+	}
+
+	if (length > dev->me_clients[id].props.max_msg_length) {
+		rets = -EFBIG;
 		goto out;
 	}
 
-- 
1.8.3.1

Re: [char-misc-next 5/5] mei: revamp read and write length checks

[Greg KH]

On Mon, Sep 02, 2013 at 03:11:04AM +0300, Tomas Winkler wrote:
> 1. Return zero on zero length read and writes
> 2. For a too large write return -EFBIG as defined in man write(2)
> EFBIG  An attempt was made to write a file that
>         exceeds the implementation-defined maximum
>         file size or the process's file size limit,
>         or to  write  at  a  position  past  the  maximum
>         allowed offset.
> 
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
>  drivers/misc/mei/main.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
> index 5ff810b..7404584 100644
> --- a/drivers/misc/mei/main.c
> +++ b/drivers/misc/mei/main.c
> @@ -203,12 +203,18 @@ static ssize_t mei_read(struct file *file, char __user *ubuf,
>  
>  	dev = cl->dev;
>  
> +
>  	mutex_lock(&dev->device_lock);

Not a big deal, but why the extra line?

greg k-h