Commit 2e7adcd5 authored by Tomas Winkler's avatar Tomas Winkler Committed by Ben Hutchings

mei: fix possible integer overflow issue

commit f862b6b2 upstream.

There is a possible integer overflow following by a buffer overflow
when accumulating messages coming from the FW to compose a full payload.
Occurrence of wrap around has to be prevented for next message size
calculation.
For unsigned integer the addition overflow has occurred when the
result is smaller than one of the arguments.
To simplify the fix, the types of buf.size and buf_idx are set to the
same width, namely size_t also to be aligned with the type of length
parameter in file read/write ops.
Signed-off-by: default avatarTomas Winkler <tomas.winkler@intel.com>
Signed-off-by: default avatarAlexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
[bwh: Backported to 3.16:
 - Adjust context, indentation
 - Return error directly, rather than through cb->status and the completion list
 - Fix up additional format string in mei_cl_write()]
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
parent edf41954
...@@ -234,9 +234,8 @@ int mei_amthif_read(struct mei_device *dev, struct file *file, ...@@ -234,9 +234,8 @@ int mei_amthif_read(struct mei_device *dev, struct file *file,
* remove message from deletion list * remove message from deletion list
*/ */
dev_dbg(&dev->pdev->dev, "amthif cb->response_buffer size - %d\n", dev_dbg(&dev->pdev->dev, "amthif cb->response_buffer.size - %zd cb->buf_idx - %zd\n",
cb->response_buffer.size); cb->response_buffer.size, cb->buf_idx);
dev_dbg(&dev->pdev->dev, "amthif cb->buf_idx - %lu\n", cb->buf_idx);
/* length is being truncated to PAGE_SIZE, however, /* length is being truncated to PAGE_SIZE, however,
* the buf_idx may point beyond */ * the buf_idx may point beyond */
......
...@@ -849,7 +849,7 @@ int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb, ...@@ -849,7 +849,7 @@ int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb,
return 0; return 0;
} }
cl_dbg(dev, cl, "buf: size = %d idx = %lu\n", cl_dbg(dev, cl, "buf: size = %zd idx = %zd\n",
cb->request_buffer.size, cb->buf_idx); cb->request_buffer.size, cb->buf_idx);
rets = mei_write_message(dev, &mei_hdr, buf->data + cb->buf_idx); rets = mei_write_message(dev, &mei_hdr, buf->data + cb->buf_idx);
...@@ -900,7 +900,7 @@ int mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb, bool blocking) ...@@ -900,7 +900,7 @@ int mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb, bool blocking)
buf = &cb->request_buffer; buf = &cb->request_buffer;
cl_dbg(dev, cl, "mei_cl_write %d\n", buf->size); cl_dbg(dev, cl, "mei_cl_write %zu\n", buf->size);
rets = pm_runtime_get(&dev->pdev->dev); rets = pm_runtime_get(&dev->pdev->dev);
if (rets < 0 && rets != -EINPROGRESS) { if (rets < 0 && rets != -EINPROGRESS) {
......
...@@ -102,6 +102,7 @@ static int mei_cl_irq_read_msg(struct mei_device *dev, ...@@ -102,6 +102,7 @@ static int mei_cl_irq_read_msg(struct mei_device *dev,
struct mei_cl *cl; struct mei_cl *cl;
struct mei_cl_cb *cb, *next; struct mei_cl_cb *cb, *next;
unsigned char *buffer = NULL; unsigned char *buffer = NULL;
size_t buf_sz;
list_for_each_entry_safe(cb, next, &dev->read_list.list, list) { list_for_each_entry_safe(cb, next, &dev->read_list.list, list) {
cl = cb->cl; cl = cb->cl;
...@@ -117,13 +118,21 @@ static int mei_cl_irq_read_msg(struct mei_device *dev, ...@@ -117,13 +118,21 @@ static int mei_cl_irq_read_msg(struct mei_device *dev,
return -ENOMEM; return -ENOMEM;
} }
if (cb->response_buffer.size < mei_hdr->length + cb->buf_idx) { buf_sz = mei_hdr->length + cb->buf_idx;
cl_dbg(dev, cl, "message overflow. size %d len %d idx %ld\n", /* catch for integer overflow */
if (buf_sz < cb->buf_idx) {
cl_err(dev, cl, "message is too big len %d idx %ld\n",
mei_hdr->length, cb->buf_idx);
list_del(&cb->list);
return -EMSGSIZE;
}
if (cb->response_buffer.size < buf_sz) {
cl_dbg(dev, cl, "message overflow. size %zd len %d idx %zd\n",
cb->response_buffer.size, cb->response_buffer.size,
mei_hdr->length, cb->buf_idx); mei_hdr->length, cb->buf_idx);
buffer = krealloc(cb->response_buffer.data, buffer = krealloc(cb->response_buffer.data, buf_sz, GFP_KERNEL);
mei_hdr->length + cb->buf_idx,
GFP_KERNEL);
if (!buffer) { if (!buffer) {
cl_err(dev, cl, "allocation failed.\n"); cl_err(dev, cl, "allocation failed.\n");
...@@ -131,8 +140,7 @@ static int mei_cl_irq_read_msg(struct mei_device *dev, ...@@ -131,8 +140,7 @@ static int mei_cl_irq_read_msg(struct mei_device *dev,
return -ENOMEM; return -ENOMEM;
} }
cb->response_buffer.data = buffer; cb->response_buffer.data = buffer;
cb->response_buffer.size = cb->response_buffer.size = buf_sz;
mei_hdr->length + cb->buf_idx;
} }
buffer = cb->response_buffer.data + cb->buf_idx; buffer = cb->response_buffer.data + cb->buf_idx;
......
...@@ -262,7 +262,7 @@ static ssize_t mei_read(struct file *file, char __user *ubuf, ...@@ -262,7 +262,7 @@ static ssize_t mei_read(struct file *file, char __user *ubuf,
} }
/* now copy the data to user space */ /* now copy the data to user space */
copy_buffer: copy_buffer:
dev_dbg(&dev->pdev->dev, "buf.size = %d buf.idx= %ld\n", dev_dbg(&dev->pdev->dev, "buf.size = %zd buf.idx = %zd\n",
cb->response_buffer.size, cb->buf_idx); cb->response_buffer.size, cb->buf_idx);
if (length == 0 || ubuf == NULL || *offset > cb->buf_idx) { if (length == 0 || ubuf == NULL || *offset > cb->buf_idx) {
rets = -EMSGSIZE; rets = -EMSGSIZE;
...@@ -281,7 +281,8 @@ static ssize_t mei_read(struct file *file, char __user *ubuf, ...@@ -281,7 +281,8 @@ static ssize_t mei_read(struct file *file, char __user *ubuf,
rets = length; rets = length;
*offset += length; *offset += length;
if ((unsigned long)*offset < cb->buf_idx) /* not all data was read, keep the cb */
if (*offset < cb->buf_idx)
goto out; goto out;
free: free:
......
...@@ -149,7 +149,7 @@ enum mei_cb_file_ops { ...@@ -149,7 +149,7 @@ enum mei_cb_file_ops {
* Intel MEI message data struct * Intel MEI message data struct
*/ */
struct mei_msg_data { struct mei_msg_data {
u32 size; size_t size;
unsigned char *data; unsigned char *data;
}; };
...@@ -195,7 +195,7 @@ struct mei_cl_cb { ...@@ -195,7 +195,7 @@ struct mei_cl_cb {
enum mei_cb_file_ops fop_type; enum mei_cb_file_ops fop_type;
struct mei_msg_data request_buffer; struct mei_msg_data request_buffer;
struct mei_msg_data response_buffer; struct mei_msg_data response_buffer;
unsigned long buf_idx; size_t buf_idx;
unsigned long read_time; unsigned long read_time;
struct file *file_object; struct file *file_object;
u32 internal:1; u32 internal:1;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment