From: Tomas Winkler <tomas.winkler@intel.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Alexander Usyskin <alexander.usyskin@intel.com>,
linux-kernel@vger.kernel.org,
Tomas Winkler <tomas.winkler@intel.com>
Subject: [char-misc-next 03/27] mei: fix possible integer overflow issue
Date: Sun, 7 Feb 2016 23:35:19 +0200 [thread overview]
Message-ID: <1454880943-12653-4-git-send-email-tomas.winkler@intel.com> (raw)
In-Reply-To: <1454880943-12653-1-git-send-email-tomas.winkler@intel.com>
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>
---
drivers/misc/mei/amthif.c | 5 ++---
drivers/misc/mei/client.c | 2 +-
drivers/misc/mei/interrupt.c | 21 ++++++++++++++++-----
drivers/misc/mei/main.c | 5 +++--
drivers/misc/mei/mei_dev.h | 4 ++--
5 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/misc/mei/amthif.c b/drivers/misc/mei/amthif.c
index cd0403f09267..b753df98b476 100644
--- 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 e069fcaed7aa..738f3d703323 100644
--- 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 6340dee33052..b8aa047ec258 100644
--- 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 55ae4789c439..5c530bab97f1 100644
--- 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 da613268480c..9b793f87b7d4 100644
--- 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;
--
2.4.3
next prev parent reply other threads:[~2016-02-07 21:44 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-07 21:35 [char-misc-next 00/27] mei: fixes, improvements, and cleanups Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 01/27] mei: debugfs: adjust active clients print buffer Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 02/27] mei: debugfs: allow hbm features list dump in earlier stages Tomas Winkler
2016-02-07 21:35 ` Tomas Winkler [this message]
2016-02-07 21:35 ` [char-misc-next 04/27] mei: call stop on failed char device register Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 05/27] mei: amthif: don't copy from an empty buffer Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 06/27] mei: amthif: don't drop read packets on timeout Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 07/27] mei: constify struct file pointer Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 08/27] mei: rename variable names 'file_object' to fp Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 09/27] mei: amthif: allow only one request at a time Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 10/27] mei: amthif: replace amthif_rd_complete_list with rd_completed Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 11/27] mei: amthif: drop parameter validation from mei_amthif_write Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 12/27] mei: amthif: use rx_wait queue also for amthif client Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 13/27] mei: amthif: interrupt reader on link reset Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 14/27] mei: bus: fix RX event scheduling Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 15/27] mei: bus: fix notification event delivery Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 16/27] mei: bus: check if the device is enabled before data transfer Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 17/27] mei: drop superfluous closing bracket from write traces Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 18/27] mei: wake blocked write on link reset Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 19/27] mei: clean write queues and wake waiters on disconnect Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 20/27] mei: discard replies from unconnected fixed address clients Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 21/27] mei: fill file pointer in read cb for fixed address client Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 22/27] mei: fixed address clients for the new platforms Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 23/27] mei: hbm: warn about fw-initiated disconnect Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 24/27] mei: drop reserved host client ids Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 25/27] mei: bus: run rescan on me_clients list change Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 26/27] mei: hbm: send immediate reply flag in enum request Tomas Winkler
2016-02-07 21:35 ` [char-misc-next 27/27] mei: split amthif client init from end of clients enumeration Tomas Winkler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1454880943-12653-4-git-send-email-tomas.winkler@intel.com \
--to=tomas.winkler@intel.com \
--cc=alexander.usyskin@intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).