From 77ad483f7b82d944aae5b944cd28e923a5293668 Mon Sep 17 00:00:00 2001 From: Ravi Aravamudhan Date: Thu, 15 Nov 2012 16:04:04 -0800 Subject: diag: Improve handling of IOCTLs DIAG kernel driver interacts with user space processes using IOCTLS. This change adds conditions to avoid potential integer over/underflow, incorrect buffer copy. CVE-2012-4220 CVE-2012-4221 Change-Id: Ic1e815051ae9544c911c9a5bd0c9218c1225f6d5 CRs-Fixed: 385352 CRs-Fixed: 385349 Signed-off-by: Shalabh Jain --- drivers/char/diag/diagchar.h | 1 + drivers/char/diag/diagchar_core.c | 188 ++++++++++++++++++++++++++++---------- 2 files changed, 142 insertions(+), 47 deletions(-) diff --git a/drivers/char/diag/diagchar.h b/drivers/char/diag/diagchar.h index 28d0565..de3cf522 100644 --- a/drivers/char/diag/diagchar.h +++ b/drivers/char/diag/diagchar.h @@ -29,6 +29,7 @@ #define IN_BUF_SIZE 16384 #define MAX_IN_BUF_SIZE 32768 #define MAX_SYNC_OBJ_NAME_SIZE 32 +#define UINT32_MAX UINT_MAX /* Size of the buffer used for deframing a packet reveived from the PC tool*/ #define HDLC_MAX 4096 diff --git a/drivers/char/diag/diagchar_core.c b/drivers/char/diag/diagchar_core.c index 19c6ed2..7b17ce4 100644 --- a/drivers/char/diag/diagchar_core.c +++ b/drivers/char/diag/diagchar_core.c @@ -358,7 +358,7 @@ void diag_clear_reg(int proc_num) } void diag_add_reg(int j, struct bindpkt_params *params, - int *success, int *count_entries) + int *success, unsigned int *count_entries) { *success = 1; driver->table[j].cmd_code = params->cmd_code; @@ -399,79 +399,153 @@ inline uint16_t diag_get_remote_device_mask(void) { return 0; } long diagchar_ioctl(struct file *filp, unsigned int iocmd, unsigned long ioarg) { - int i, j, count_entries = 0, temp; - int success = -1; + int i, j, temp, success = -1, status; + unsigned int count_entries = 0, interim_count = 0; void *temp_buf; uint16_t support_list = 0; - struct diag_dci_client_tbl *params = - kzalloc(sizeof(struct diag_dci_client_tbl), GFP_KERNEL); + struct diag_dci_client_tbl *dci_params; struct diag_dci_health_stats stats; - int status; if (iocmd == DIAG_IOCTL_COMMAND_REG) { - struct bindpkt_params_per_process *pkt_params = - (struct bindpkt_params_per_process *) ioarg; + struct bindpkt_params_per_process pkt_params; + struct bindpkt_params *params; + struct bindpkt_params *head_params; + if (copy_from_user(&pkt_params, (void *)ioarg, + sizeof(struct bindpkt_params_per_process))) { + return -EFAULT; + } + if ((UINT32_MAX/sizeof(struct bindpkt_params)) < + pkt_params.count) { + pr_warning("diag: integer overflow while multiply\n"); + return -EFAULT; + } + params = kzalloc(pkt_params.count*sizeof( + struct bindpkt_params), GFP_KERNEL); + if (!params) { + pr_err("diag: unable to alloc memory\n"); + return -ENOMEM; + } else + head_params = params; + + if (copy_from_user(params, pkt_params.params, + pkt_params.count*sizeof(struct bindpkt_params))) { + kfree(head_params); + return -EFAULT; + } mutex_lock(&driver->diagchar_mutex); for (i = 0; i < diag_max_reg; i++) { if (driver->table[i].process_id == 0) { - diag_add_reg(i, pkt_params->params, - &success, &count_entries); - if (pkt_params->count > count_entries) { - pkt_params->params++; + diag_add_reg(i, params, &success, + &count_entries); + if (pkt_params.count > count_entries) { + params++; } else { mutex_unlock(&driver->diagchar_mutex); + kfree(head_params); return success; } } } if (i < diag_threshold_reg) { /* Increase table size by amount required */ - diag_max_reg += pkt_params->count - + if (pkt_params.count >= count_entries) { + interim_count = pkt_params.count - count_entries; + } else { + pr_warning("diag: error in params count\n"); + kfree(head_params); + mutex_unlock(&driver->diagchar_mutex); + return -EFAULT; + } + if (UINT32_MAX - diag_max_reg >= + interim_count) { + diag_max_reg += interim_count; + } else { + pr_warning("diag: Integer overflow\n"); + kfree(head_params); + mutex_unlock(&driver->diagchar_mutex); + return -EFAULT; + } /* Make sure size doesnt go beyond threshold */ if (diag_max_reg > diag_threshold_reg) { diag_max_reg = diag_threshold_reg; pr_info("diag: best case memory allocation\n"); } + if (UINT32_MAX/sizeof(struct diag_master_table) < + diag_max_reg) { + pr_warning("diag: integer overflow\n"); + kfree(head_params); + mutex_unlock(&driver->diagchar_mutex); + return -EFAULT; + } temp_buf = krealloc(driver->table, diag_max_reg*sizeof(struct diag_master_table), GFP_KERNEL); if (!temp_buf) { - diag_max_reg -= pkt_params->count - - count_entries; - pr_alert("diag: Insufficient memory for reg."); + pr_alert("diag: Insufficient memory for reg.\n"); mutex_unlock(&driver->diagchar_mutex); + + if (pkt_params.count >= count_entries) { + interim_count = pkt_params.count - + count_entries; + } else { + pr_warning("diag: params count error\n"); + mutex_unlock(&driver->diagchar_mutex); + kfree(head_params); + return -EFAULT; + } + if (diag_max_reg >= interim_count) { + diag_max_reg -= interim_count; + } else { + pr_warning("diag: Integer underflow\n"); + mutex_unlock(&driver->diagchar_mutex); + kfree(head_params); + return -EFAULT; + } + kfree(head_params); return 0; } else { driver->table = temp_buf; } for (j = i; j < diag_max_reg; j++) { - diag_add_reg(j, pkt_params->params, - &success, &count_entries); - if (pkt_params->count > count_entries) { - pkt_params->params++; + diag_add_reg(j, params, &success, + &count_entries); + if (pkt_params.count > count_entries) { + params++; } else { mutex_unlock(&driver->diagchar_mutex); + kfree(head_params); return success; } } + kfree(head_params); mutex_unlock(&driver->diagchar_mutex); } else { mutex_unlock(&driver->diagchar_mutex); + kfree(head_params); pr_err("Max size reached, Pkt Registration failed for" " Process %d", current->tgid); } success = 0; } else if (iocmd == DIAG_IOCTL_GET_DELAYED_RSP_ID) { - struct diagpkt_delay_params *delay_params = - (struct diagpkt_delay_params *) ioarg; - - if ((delay_params->rsp_ptr) && - (delay_params->size == sizeof(delayed_rsp_id)) && - (delay_params->num_bytes_ptr)) { - *((uint16_t *)delay_params->rsp_ptr) = - DIAGPKT_NEXT_DELAYED_RSP_ID(delayed_rsp_id); - *(delay_params->num_bytes_ptr) = sizeof(delayed_rsp_id); + struct diagpkt_delay_params delay_params; + uint16_t interim_rsp_id; + int interim_size; + if (copy_from_user(&delay_params, (void *)ioarg, + sizeof(struct diagpkt_delay_params))) + return -EFAULT; + if ((delay_params.rsp_ptr) && + (delay_params.size == sizeof(delayed_rsp_id)) && + (delay_params.num_bytes_ptr)) { + interim_rsp_id = DIAGPKT_NEXT_DELAYED_RSP_ID( + delayed_rsp_id); + if (copy_to_user((void *)delay_params.rsp_ptr, + &interim_rsp_id, sizeof(uint16_t))) + return -EFAULT; + interim_size = sizeof(delayed_rsp_id); + if (copy_to_user((void *)delay_params.num_bytes_ptr, + &interim_size, sizeof(int))) + return -EFAULT; success = 0; } } else if (iocmd == DIAG_IOCTL_DCI_REG) { @@ -479,7 +553,13 @@ long diagchar_ioctl(struct file *filp, return DIAG_DCI_NO_REG; if (driver->num_dci_client >= MAX_DCI_CLIENTS) return DIAG_DCI_NO_REG; - if (copy_from_user(params, (void *)ioarg, + dci_params = kzalloc(sizeof(struct diag_dci_client_tbl), + GFP_KERNEL); + if (dci_params == NULL) { + pr_err("diag: unable to alloc memory\n"); + return -ENOMEM; + } + if (copy_from_user(dci_params, (void *)ioarg, sizeof(struct diag_dci_client_tbl))) return -EFAULT; mutex_lock(&driver->dci_mutex); @@ -492,9 +572,9 @@ long diagchar_ioctl(struct file *filp, if (driver->dci_client_tbl[i].client == NULL) { driver->dci_client_tbl[i].client = current; driver->dci_client_tbl[i].list = - params->list; + dci_params->list; driver->dci_client_tbl[i].signal_type = - params->signal_type; + dci_params->signal_type; create_dci_log_mask_tbl(driver-> dci_client_tbl[i].dci_log_mask); create_dci_event_mask_tbl(driver-> @@ -512,6 +592,7 @@ long diagchar_ioctl(struct file *filp, } } mutex_unlock(&driver->dci_mutex); + kfree(dci_params); return driver->dci_client_id; } else if (iocmd == DIAG_IOCTL_DCI_DEINIT) { success = -1; @@ -536,25 +617,29 @@ long diagchar_ioctl(struct file *filp, } else if (iocmd == DIAG_IOCTL_DCI_SUPPORT) { if (driver->ch_dci) support_list = support_list | DIAG_CON_MPSS; - *(uint16_t *)ioarg = support_list; + if (copy_to_user((void *)ioarg, &support_list, + sizeof(uint16_t))) + return -EFAULT; return DIAG_DCI_NO_ERROR; } else if (iocmd == DIAG_IOCTL_DCI_HEALTH_STATS) { if (copy_from_user(&stats, (void *)ioarg, sizeof(struct diag_dci_health_stats))) return -EFAULT; for (i = 0; i < MAX_DCI_CLIENTS; i++) { - params = &(driver->dci_client_tbl[i]); - if (params->client && - params->client->tgid == current->tgid) { - stats.dropped_logs = params->dropped_logs; - stats.dropped_events = params->dropped_events; - stats.received_logs = params->received_logs; - stats.received_events = params->received_events; + dci_params = &(driver->dci_client_tbl[i]); + if (dci_params->client && + dci_params->client->tgid == current->tgid) { + stats.dropped_logs = dci_params->dropped_logs; + stats.dropped_events = + dci_params->dropped_events; + stats.received_logs = dci_params->received_logs; + stats.received_events = + dci_params->received_events; if (stats.reset_status) { - params->dropped_logs = 0; - params->dropped_events = 0; - params->received_logs = 0; - params->received_events = 0; + dci_params->dropped_logs = 0; + dci_params->dropped_events = 0; + dci_params->received_logs = 0; + dci_params->received_events = 0; } break; } @@ -567,7 +652,7 @@ long diagchar_ioctl(struct file *filp, for (i = 0; i < driver->num_clients; i++) if (driver->client_map[i].pid == current->tgid) break; - if (i == -1) + if (i == driver->num_clients) return -EINVAL; driver->data_ready[i] |= DEINIT_TYPE; wake_up_interruptible(&driver->wait_q); @@ -1068,7 +1153,7 @@ static int diagchar_write(struct file *file, const char __user *buf, struct diag_send_desc_type send = { NULL, NULL, DIAG_STATE_START, 0 }; struct diag_hdlc_dest_type enc = { NULL, NULL, 0 }; void *buf_copy = NULL; - int payload_size; + unsigned int payload_size; #ifdef CONFIG_DIAG_OVER_USB if (((driver->logging_mode == USB_MODE) && (!driver->usb_connected)) || (driver->logging_mode == NO_LOGGING_MODE)) { @@ -1079,8 +1164,17 @@ static int diagchar_write(struct file *file, const char __user *buf, /* Get the packet type F3/log/event/Pkt response */ err = copy_from_user((&pkt_type), buf, 4); /* First 4 bytes indicate the type of payload - ignore these */ + if (count < 4) { + pr_err("diag: Client sending short data\n"); + return -EBADMSG; + } payload_size = count - 4; - + if (payload_size > USER_SPACE_DATA) { + pr_err("diag: Dropping packet, packet payload size crosses 8KB limit. Current payload size %d\n", + payload_size); + driver->dropped_count++; + return -EBADMSG; + } if (pkt_type == DCI_DATA_TYPE) { err = copy_from_user(driver->user_space_data, buf + 4, payload_size); -- cgit v1.1