From 75eac48a48562f819f50eeff8369b296d89102d7 Mon Sep 17 00:00:00 2001 From: Katish Paran Date: Tue, 24 Dec 2013 17:46:29 +0530 Subject: diag: Safeguard for bound checks and integer underflow At certain point in diag driver there can be integer underflow and thus can lead to memory leak. Bound checks are placed to ensure correct behavior of condition statements. Change-Id: I47e02f764c2c7412db6f90fd42192fee32a761d3 CRs-fixed: 549470 Signed-off-by: Katish Paran --- drivers/char/diag/diag_debugfs.c | 15 ++++++++------- drivers/char/diag/diagchar_core.c | 15 +++++++++++++-- drivers/char/diag/diagchar_hdlc.c | 4 ++-- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/drivers/char/diag/diag_debugfs.c b/drivers/char/diag/diag_debugfs.c index 4bbe948..d63d34b 100644 --- a/drivers/char/diag/diag_debugfs.c +++ b/drivers/char/diag/diag_debugfs.c @@ -197,7 +197,8 @@ static ssize_t diag_dbgfs_read_dcistats(struct file *file, char __user *ubuf, size_t count, loff_t *ppos) { char *buf = NULL; - int bytes_remaining, bytes_written = 0, bytes_in_buf = 0, i = 0; + unsigned int bytes_remaining, bytes_written = 0; + unsigned int bytes_in_buf = 0, i = 0; struct diag_dci_data_info *temp_data = dci_data_smd; int buf_size = (DEBUG_BUF_SIZE < count) ? DEBUG_BUF_SIZE : count; @@ -353,9 +354,9 @@ static ssize_t diag_dbgfs_read_table(struct file *file, char __user *ubuf, char *buf; int ret = 0; int i; - int bytes_remaining; - int bytes_in_buffer = 0; - int bytes_written; + unsigned int bytes_remaining; + unsigned int bytes_in_buffer = 0; + unsigned int bytes_written; int buf_size = (DEBUG_BUF_SIZE < count) ? DEBUG_BUF_SIZE : count; if (diag_dbgfs_table_index >= diag_max_reg) { @@ -526,9 +527,9 @@ static ssize_t diag_dbgfs_read_bridge(struct file *file, char __user *ubuf, char *buf; int ret; int i; - int bytes_remaining; - int bytes_in_buffer = 0; - int bytes_written; + unsigned int bytes_remaining; + unsigned int bytes_in_buffer = 0; + unsigned int bytes_written; int buf_size = (DEBUG_BUF_SIZE < count) ? DEBUG_BUF_SIZE : count; int bytes_hsic_inited = 45; int bytes_hsic_not_inited = 410; diff --git a/drivers/char/diag/diagchar_core.c b/drivers/char/diag/diagchar_core.c index 38ca47b..ab68902 100644 --- a/drivers/char/diag/diagchar_core.c +++ b/drivers/char/diag/diagchar_core.c @@ -51,6 +51,7 @@ MODULE_DESCRIPTION("Diag Char Driver"); MODULE_LICENSE("GPL v2"); MODULE_VERSION("1.0"); +#define MIN_SIZ_ALLOW 4 #define INIT 1 #define EXIT -1 struct diagchar_dev *driver; @@ -1461,6 +1462,10 @@ static ssize_t diagchar_write(struct file *file, const char __user *buf, index = 0; /* Get the packet type F3/log/event/Pkt response */ err = copy_from_user((&pkt_type), buf, 4); + if (err) { + pr_alert("diag: copy failed for pkt_type\n"); + return -EAGAIN; + } /* First 4 bytes indicate the type of payload - ignore these */ if (count < 4) { pr_err("diag: Client sending short data\n"); @@ -1504,8 +1509,9 @@ static ssize_t diagchar_write(struct file *file, const char __user *buf, return err; } if (pkt_type == CALLBACK_DATA_TYPE) { - if (payload_size > driver->itemsize) { - pr_err("diag: Dropping packet, packet payload size crosses 4KB limit. Current payload size %d\n", + if (payload_size > driver->itemsize || + payload_size <= MIN_SIZ_ALLOW) { + pr_err("diag: Dropping packet, invalid packet size. Current payload size %d\n", payload_size); driver->dropped_count++; return -EBADMSG; @@ -1639,6 +1645,11 @@ static ssize_t diagchar_write(struct file *file, const char __user *buf, diag_get_remote(*(int *)driver->user_space_data_buf); if (remote_proc) { + if (payload_size <= MIN_SIZ_ALLOW) { + pr_err("diag: Integer underflow in %s, payload size: %d", + __func__, payload_size); + return -EBADMSG; + } token_offset = 4; payload_size -= 4; buf += 4; diff --git a/drivers/char/diag/diagchar_hdlc.c b/drivers/char/diag/diagchar_hdlc.c index d5ba452..39f1f44 100644 --- a/drivers/char/diag/diagchar_hdlc.c +++ b/drivers/char/diag/diagchar_hdlc.c @@ -177,8 +177,8 @@ int diag_hdlc_decode(struct diag_hdlc_decode_type *hdlc) int msg_start; if (hdlc && hdlc->src_ptr && hdlc->dest_ptr && - (hdlc->src_size - hdlc->src_idx > 0) && - (hdlc->dest_size - hdlc->dest_idx > 0)) { + (hdlc->src_size > hdlc->src_idx) && + (hdlc->dest_size > hdlc->dest_idx)) { msg_start = (hdlc->src_idx == 0) ? 1 : 0; -- cgit v1.1