* [char-misc-next 1/3] mei: check for error returned from mei_hbuf_empty_slots()
2018-07-12 14:10 [char-misc-next 0/3] mei: more type conversion issues Tomas Winkler
@ 2018-07-12 14:10 ` Tomas Winkler
2018-07-12 14:10 ` [char-misc-next 2/3] mei: use correct type for counter variable in for loops Tomas Winkler
2018-07-12 14:10 ` [char-misc-next 3/3] mei: fix ssize_t to int assignment in read and write ops Tomas Winkler
2 siblings, 0 replies; 4+ messages in thread
From: Tomas Winkler @ 2018-07-12 14:10 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Alexander Usyskin, linux-kernel, Tomas Winkler
mei_hbuf_empty_slots() may return with an error in case
of circular buffer overflow. This type of error may
be caused only by a bug. However currently, the error
won't be detected due signed type promotion in comparison to u32.
We add explicit check for less then zero and explicit cast
in comparison to suppress singn-compare warning.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
drivers/misc/mei/client.c | 22 ++++++++++++++++------
drivers/misc/mei/hw-me.c | 5 ++++-
drivers/misc/mei/hw-txe.c | 2 +-
drivers/misc/mei/interrupt.c | 15 +++++++++++----
4 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 8d6197a88b54..f8fb7589192e 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -865,8 +865,10 @@ int mei_cl_irq_disconnect(struct mei_cl *cl, struct mei_cl_cb *cb,
msg_slots = mei_data2slots(sizeof(struct hbm_client_connect_request));
slots = mei_hbuf_empty_slots(dev);
+ if (slots < 0)
+ return -EOVERFLOW;
- if (slots < msg_slots)
+ if ((u32)slots < msg_slots)
return -EMSGSIZE;
ret = mei_cl_send_disconnect(cl, cb);
@@ -1054,12 +1056,15 @@ int mei_cl_irq_connect(struct mei_cl *cl, struct mei_cl_cb *cb,
int rets;
msg_slots = mei_data2slots(sizeof(struct hbm_client_connect_request));
- slots = mei_hbuf_empty_slots(dev);
if (mei_cl_is_other_connecting(cl))
return 0;
- if (slots < msg_slots)
+ slots = mei_hbuf_empty_slots(dev);
+ if (slots < 0)
+ return -EOVERFLOW;
+
+ if ((u32)slots < msg_slots)
return -EMSGSIZE;
rets = mei_cl_send_connect(cl, cb);
@@ -1296,8 +1301,10 @@ int mei_cl_irq_notify(struct mei_cl *cl, struct mei_cl_cb *cb,
msg_slots = mei_data2slots(sizeof(struct hbm_client_connect_request));
slots = mei_hbuf_empty_slots(dev);
+ if (slots < 0)
+ return -EOVERFLOW;
- if (slots < msg_slots)
+ if ((u32)slots < msg_slots)
return -EMSGSIZE;
request = mei_cl_notify_fop2req(cb->fop_type);
@@ -1573,6 +1580,9 @@ int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb,
}
slots = mei_hbuf_empty_slots(dev);
+ if (slots < 0)
+ return -EOVERFLOW;
+
len = buf->size - cb->buf_idx;
msg_slots = mei_data2slots(len);
@@ -1581,11 +1591,11 @@ int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb,
mei_hdr.reserved = 0;
mei_hdr.internal = cb->internal;
- if (slots >= msg_slots) {
+ if ((u32)slots >= msg_slots) {
mei_hdr.length = len;
mei_hdr.msg_complete = 1;
/* Split the message only if we can write the whole host buffer */
- } else if (slots == dev->hbuf_depth) {
+ } else if ((u32)slots == dev->hbuf_depth) {
msg_slots = slots;
len = (slots * sizeof(u32)) - sizeof(struct mei_msg_hdr);
mei_hdr.length = len;
diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index 334ab02e1de2..a12b464bc0b4 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -540,8 +540,11 @@ static int mei_me_hbuf_write(struct mei_device *dev,
empty_slots = mei_hbuf_empty_slots(dev);
dev_dbg(dev->dev, "empty slots = %hu.\n", empty_slots);
+ if (empty_slots < 0)
+ return -EOVERFLOW;
+
dw_cnt = mei_data2slots(length);
- if (empty_slots < 0 || dw_cnt > empty_slots)
+ if (dw_cnt > (u32)empty_slots)
return -EMSGSIZE;
mei_me_hcbww_write(dev, *((u32 *) header));
diff --git a/drivers/misc/mei/hw-txe.c b/drivers/misc/mei/hw-txe.c
index c2c8993e2a51..4c1acf68089e 100644
--- a/drivers/misc/mei/hw-txe.c
+++ b/drivers/misc/mei/hw-txe.c
@@ -709,7 +709,7 @@ static int mei_txe_write(struct mei_device *dev,
struct mei_txe_hw *hw = to_txe_hw(dev);
unsigned long rem;
unsigned long length;
- int slots = dev->hbuf_depth;
+ u32 slots = dev->hbuf_depth;
u32 *reg_buf = (u32 *)buf;
u32 dw_cnt;
int i;
diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index 6649f0d56d2f..6217cebcad3d 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -173,10 +173,12 @@ static int mei_cl_irq_disconnect_rsp(struct mei_cl *cl, struct mei_cl_cb *cb,
int slots;
int ret;
- slots = mei_hbuf_empty_slots(dev);
msg_slots = mei_data2slots(sizeof(struct hbm_client_connect_response));
+ slots = mei_hbuf_empty_slots(dev);
+ if (slots < 0)
+ return -EOVERFLOW;
- if (slots < msg_slots)
+ if ((u32)slots < msg_slots)
return -EMSGSIZE;
ret = mei_hbm_cl_disconnect_rsp(dev, cl);
@@ -208,8 +210,10 @@ static int mei_cl_irq_read(struct mei_cl *cl, struct mei_cl_cb *cb,
msg_slots = mei_data2slots(sizeof(struct hbm_flow_control));
slots = mei_hbuf_empty_slots(dev);
+ if (slots < 0)
+ return -EOVERFLOW;
- if (slots < msg_slots)
+ if ((u32)slots < msg_slots)
return -EMSGSIZE;
ret = mei_hbm_cl_flow_control_req(dev, cl);
@@ -368,7 +372,10 @@ int mei_irq_write_handler(struct mei_device *dev, struct list_head *cmpl_list)
return 0;
slots = mei_hbuf_empty_slots(dev);
- if (slots <= 0)
+ if (slots < 0)
+ return -EOVERFLOW;
+
+ if (slots == 0)
return -EMSGSIZE;
/* complete all waiting for write CB */
--
2.14.4
^ permalink raw reply [flat|nested] 4+ messages in thread* [char-misc-next 3/3] mei: fix ssize_t to int assignment in read and write ops.
2018-07-12 14:10 [char-misc-next 0/3] mei: more type conversion issues Tomas Winkler
2018-07-12 14:10 ` [char-misc-next 1/3] mei: check for error returned from mei_hbuf_empty_slots() Tomas Winkler
2018-07-12 14:10 ` [char-misc-next 2/3] mei: use correct type for counter variable in for loops Tomas Winkler
@ 2018-07-12 14:10 ` Tomas Winkler
2 siblings, 0 replies; 4+ messages in thread
From: Tomas Winkler @ 2018-07-12 14:10 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Alexander Usyskin, linux-kernel, Tomas Winkler
Use ssize_t for rets variables in mei_write(), mei_read(), and
mei_cl_write() as well as change the return type of mei_cl_write()
to ssize_t, to prevent assignment of possible 64bit size_t
to int 32 bit variable.
As by product also eliminate warning
drivers/misc/mei/client.c:1702:11: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
drivers/misc/mei/client.c | 22 +++++++++++-----------
drivers/misc/mei/client.h | 2 +-
drivers/misc/mei/main.c | 10 +++++-----
3 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index f8fb7589192e..5a673d09585f 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -1644,13 +1644,13 @@ int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb,
*
* Return: number of bytes sent on success, <0 on failure.
*/
-int mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb)
+ssize_t mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb)
{
struct mei_device *dev;
struct mei_msg_data *buf;
struct mei_msg_hdr mei_hdr;
- int size;
- int rets;
+ size_t len;
+ ssize_t rets;
bool blocking;
if (WARN_ON(!cl || !cl->dev))
@@ -1662,15 +1662,15 @@ int mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb)
dev = cl->dev;
buf = &cb->buf;
- size = buf->size;
+ len = buf->size;
blocking = cb->blocking;
- cl_dbg(dev, cl, "size=%d\n", size);
+ cl_dbg(dev, cl, "len=%zd\n", len);
rets = pm_runtime_get(dev->dev);
if (rets < 0 && rets != -EINPROGRESS) {
pm_runtime_put_noidle(dev->dev);
- cl_err(dev, cl, "rpm: get failed %d\n", rets);
+ cl_err(dev, cl, "rpm: get failed %zd\n", rets);
goto free;
}
@@ -1689,21 +1689,21 @@ int mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb)
if (rets == 0) {
cl_dbg(dev, cl, "No flow control credentials: not sending.\n");
- rets = size;
+ rets = len;
goto out;
}
if (!mei_hbuf_acquire(dev)) {
cl_dbg(dev, cl, "Cannot acquire the host buffer: not sending.\n");
- rets = size;
+ rets = len;
goto out;
}
/* Check for a maximum length */
- if (size > mei_hbuf_max_len(dev)) {
+ if (len > mei_hbuf_max_len(dev)) {
mei_hdr.length = mei_hbuf_max_len(dev);
mei_hdr.msg_complete = 0;
} else {
- mei_hdr.length = size;
+ mei_hdr.length = len;
mei_hdr.msg_complete = 1;
}
@@ -1745,7 +1745,7 @@ int mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb)
}
}
- rets = size;
+ rets = len;
err:
cl_dbg(dev, cl, "rpm: autosuspend\n");
pm_runtime_mark_last_busy(dev->dev);
diff --git a/drivers/misc/mei/client.h b/drivers/misc/mei/client.h
index 5371df4d8af3..64e318f589b4 100644
--- a/drivers/misc/mei/client.h
+++ b/drivers/misc/mei/client.h
@@ -202,7 +202,7 @@ int mei_cl_connect(struct mei_cl *cl, struct mei_me_client *me_cl,
int mei_cl_irq_connect(struct mei_cl *cl, struct mei_cl_cb *cb,
struct list_head *cmpl_list);
int mei_cl_read_start(struct mei_cl *cl, size_t length, const struct file *fp);
-int mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb);
+ssize_t mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb);
int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb,
struct list_head *cmpl_list);
diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 302ba7a63bd2..4d77a6ae183a 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -137,7 +137,7 @@ static ssize_t mei_read(struct file *file, char __user *ubuf,
struct mei_device *dev;
struct mei_cl_cb *cb = NULL;
bool nonblock = !!(file->f_flags & O_NONBLOCK);
- int rets;
+ ssize_t rets;
if (WARN_ON(!cl || !cl->dev))
return -ENODEV;
@@ -170,7 +170,7 @@ static ssize_t mei_read(struct file *file, char __user *ubuf,
rets = mei_cl_read_start(cl, length, file);
if (rets && rets != -EBUSY) {
- cl_dbg(dev, cl, "mei start read failure status = %d\n", rets);
+ cl_dbg(dev, cl, "mei start read failure status = %zd\n", rets);
goto out;
}
@@ -204,7 +204,7 @@ static ssize_t mei_read(struct file *file, char __user *ubuf,
/* now copy the data to user space */
if (cb->status) {
rets = cb->status;
- cl_dbg(dev, cl, "read operation failed %d\n", rets);
+ cl_dbg(dev, cl, "read operation failed %zd\n", rets);
goto free;
}
@@ -236,7 +236,7 @@ static ssize_t mei_read(struct file *file, char __user *ubuf,
*offset = 0;
out:
- cl_dbg(dev, cl, "end mei read rets = %d\n", rets);
+ cl_dbg(dev, cl, "end mei read rets = %zd\n", rets);
mutex_unlock(&dev->device_lock);
return rets;
}
@@ -256,7 +256,7 @@ static ssize_t mei_write(struct file *file, const char __user *ubuf,
struct mei_cl *cl = file->private_data;
struct mei_cl_cb *cb;
struct mei_device *dev;
- int rets;
+ ssize_t rets;
if (WARN_ON(!cl || !cl->dev))
return -ENODEV;
--
2.14.4
^ permalink raw reply [flat|nested] 4+ messages in thread