Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932118AbcDZXYz (ORCPT ); Tue, 26 Apr 2016 19:24:55 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:40885 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753676AbcDZXYw (ORCPT ); Tue, 26 Apr 2016 19:24:52 -0400 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 From: Ben Hutchings To: linux-kernel@vger.kernel.org, stable@vger.kernel.org CC: akpm@linux-foundation.org, "Alexander Usyskin" , "Tomas Winkler" , "Greg Kroah-Hartman" Date: Wed, 27 Apr 2016 01:02:21 +0200 Message-ID: X-Mailer: LinuxStableQueue (scripts by bwh) Subject: [PATCH 3.16 033/217] mei: fix possible integer overflow issue In-Reply-To: X-SA-Exim-Connect-IP: 2a02:8426:ae4:c500:9cba:69ae:962d:6167 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5269 Lines: 156 3.16.35-rc1 review patch. If anyone has any objections, please let me know. ------------------ From: Tomas Winkler commit f862b6b24f0ffd954633a55f39251a6873b664ca 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: Tomas Winkler Signed-off-by: Alexander Usyskin Signed-off-by: Greg Kroah-Hartman [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: Ben Hutchings --- 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(-) --- a/drivers/misc/mei/amthif.c +++ b/drivers/misc/mei/amthif.c @@ -234,9 +234,8 @@ int mei_amthif_read(struct mei_device *d * remove message from deletion list */ - dev_dbg(&dev->pdev->dev, "amthif cb->response_buffer size - %d\n", - cb->response_buffer.size); - dev_dbg(&dev->pdev->dev, "amthif cb->buf_idx - %lu\n", cb->buf_idx); + dev_dbg(&dev->pdev->dev, "amthif cb->response_buffer.size - %zd cb->buf_idx - %zd\n", + cb->response_buffer.size, cb->buf_idx); /* length is being truncated to PAGE_SIZE, however, * the buf_idx may point beyond */ --- a/drivers/misc/mei/client.c +++ b/drivers/misc/mei/client.c @@ -849,7 +849,7 @@ int mei_cl_irq_write(struct mei_cl *cl, 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); rets = mei_write_message(dev, &mei_hdr, buf->data + cb->buf_idx); @@ -900,7 +900,7 @@ int mei_cl_write(struct mei_cl *cl, stru 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); if (rets < 0 && rets != -EINPROGRESS) { --- a/drivers/misc/mei/interrupt.c +++ b/drivers/misc/mei/interrupt.c @@ -102,6 +102,7 @@ static int mei_cl_irq_read_msg(struct me struct mei_cl *cl; struct mei_cl_cb *cb, *next; unsigned char *buffer = NULL; + size_t buf_sz; list_for_each_entry_safe(cb, next, &dev->read_list.list, list) { cl = cb->cl; @@ -117,13 +118,21 @@ static int mei_cl_irq_read_msg(struct me return -ENOMEM; } - if (cb->response_buffer.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_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, mei_hdr->length, cb->buf_idx); - buffer = krealloc(cb->response_buffer.data, - mei_hdr->length + cb->buf_idx, - GFP_KERNEL); + buffer = krealloc(cb->response_buffer.data, buf_sz, GFP_KERNEL); if (!buffer) { cl_err(dev, cl, "allocation failed.\n"); @@ -131,8 +140,7 @@ static int mei_cl_irq_read_msg(struct me return -ENOMEM; } cb->response_buffer.data = buffer; - cb->response_buffer.size = - mei_hdr->length + cb->buf_idx; + cb->response_buffer.size = buf_sz; } buffer = cb->response_buffer.data + cb->buf_idx; --- a/drivers/misc/mei/main.c +++ b/drivers/misc/mei/main.c @@ -262,7 +262,7 @@ static ssize_t mei_read(struct file *fil } /* now copy the data to user space */ 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); if (length == 0 || ubuf == NULL || *offset > cb->buf_idx) { rets = -EMSGSIZE; @@ -281,7 +281,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: --- a/drivers/misc/mei/mei_dev.h +++ b/drivers/misc/mei/mei_dev.h @@ -149,7 +149,7 @@ enum mei_cb_file_ops { * Intel MEI message data struct */ struct mei_msg_data { - u32 size; + size_t size; unsigned char *data; }; @@ -195,7 +195,7 @@ struct mei_cl_cb { enum mei_cb_file_ops fop_type; struct mei_msg_data request_buffer; struct mei_msg_data response_buffer; - unsigned long buf_idx; + size_t buf_idx; unsigned long read_time; struct file *file_object; u32 internal:1;