mei: fix possible integer overflow issue

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: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/misc/mei/amthif.c b/drivers/misc/mei/amthif.c
index cd0403f0926761ec7253c53e399844198728a3ea..b753df98b4763922fbefaf377db60b55c4ce3291 100644 (file)
--- a/drivers/misc/mei/amthif.c
+++ b/drivers/misc/mei/amthif.c
@@ -195,9 +195,8 @@ int mei_amthif_read(struct mei_device *dev, struct file *file,
                 * remove message from deletion list
                 */
 
-       dev_dbg(dev->dev, "amthif cb->buf size - %d\n",
-           cb->buf.size);
-       dev_dbg(dev->dev, "amthif cb->buf_idx - %lu\n", cb->buf_idx);
+       dev_dbg(dev->dev, "amthif cb->buf.size - %zd cb->buf_idx - %zd\n",
+               cb->buf.size, cb->buf_idx);
 
        /* length is being truncated to PAGE_SIZE, however,
         * the buf_idx may point beyond */

diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index e069fcaed7aaa08ca7479732a13be59eab244481..738f3d7033233418ad7f2d69f01f65e3a362eddd 100644 (file)
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -1569,7 +1569,7 @@ int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb,
                return 0;
        }
 
-       cl_dbg(dev, cl, "buf: size = %d idx = %lu\n",
+       cl_dbg(dev, cl, "buf: size = %zd idx = %zd\n",
                        cb->buf.size, cb->buf_idx);
 
        rets = mei_write_message(dev, &mei_hdr, buf->data + cb->buf_idx);

diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index 6340dee33052180c98e8803dfe8ae876f7b33e5d..b8aa047ec2585d967d8888fee3e08d361a5018e6 100644 (file)
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -104,6 +104,7 @@ int mei_cl_irq_read_msg(struct mei_cl *cl,
        struct mei_device *dev = cl->dev;
        struct mei_cl_cb *cb;
        unsigned char *buffer = NULL;
+       size_t buf_sz;
 
        cb = list_first_entry_or_null(&cl->rd_pending, struct mei_cl_cb, list);
        if (!cb) {
@@ -124,11 +125,21 @@ int mei_cl_irq_read_msg(struct mei_cl *cl,
                goto out;
        }
 
-       if (cb->buf.size < mei_hdr->length + cb->buf_idx) {
-               cl_dbg(dev, cl, "message overflow. size %d len %d idx %ld\n",
+       buf_sz = mei_hdr->length + cb->buf_idx;
+       /* 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_move_tail(&cb->list, &complete_list->list);
+               cb->status = -EMSGSIZE;
+               goto out;
+       }
+
+       if (cb->buf.size < buf_sz) {
+               cl_dbg(dev, cl, "message overflow. size %zd len %d idx %zd\n",
                        cb->buf.size, mei_hdr->length, cb->buf_idx);
-               buffer = krealloc(cb->buf.data, mei_hdr->length + cb->buf_idx,
-                                 GFP_KERNEL);
+               buffer = krealloc(cb->buf.data, buf_sz, GFP_KERNEL);
 
                if (!buffer) {
                        cb->status = -ENOMEM;
@@ -136,7 +147,7 @@ int mei_cl_irq_read_msg(struct mei_cl *cl,
                        goto out;
                }
                cb->buf.data = buffer;
-               cb->buf.size = mei_hdr->length + cb->buf_idx;
+               cb->buf.size = buf_sz;
        }
 
        buffer = cb->buf.data + cb->buf_idx;

diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 36ca15234344aad8b415c1732edd3052000657d6..47dc6d9ae6554885e5459f9ebe750838546cc24d 100644 (file)
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -226,7 +226,7 @@ copy_buffer:
                goto free;
        }
 
-       cl_dbg(dev, cl, "buf.size = %d buf.idx = %ld offset = %lld\n",
+       cl_dbg(dev, cl, "buf.size = %zd buf.idx = %zd offset = %lld\n",
               cb->buf.size, cb->buf_idx, *offset);
        if (*offset >= cb->buf_idx) {
                rets = 0;
@@ -245,7 +245,8 @@ copy_buffer:
 
        rets = 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;
 
 free:

diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index da613268480c67917cfca25a1ffece3d700f1a3c..9b793f87b7d471d8a1b974639edf9b505412794e 100644 (file)
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -126,7 +126,7 @@ enum mei_cb_file_ops {
  * Intel MEI message data struct
  */
 struct mei_msg_data {
-       u32 size;
+       size_t size;
        unsigned char *data;
 };
 
@@ -190,7 +190,7 @@ struct mei_cl_cb {
        struct mei_cl *cl;
        enum mei_cb_file_ops fop_type;
        struct mei_msg_data buf;
-       unsigned long buf_idx;
+       size_t buf_idx;
        unsigned long read_time;
        struct file *file_object;
        int status;