From 17ef83c708be034454df7914ef5484c36515eece Mon Sep 17 00:00:00 2001 From: Jonathan Solnit Date: Fri, 1 Sep 2017 15:19:29 -0700 Subject: [PATCH] diag: Add protection while processing non-hdlc packets Currently, there is possibility of out-of-bound accesses during handling of data in non-hdlc path. The patch adds proper protection when processing non-hdlc packet information to fix the issue. Bug: 64434485 CRs-Fixed: 2029216 Change-Id: I07c466f85bd8ac08226948fea86b1d8567e68431 Signed-off-by: Hardik Arya Signed-off-by: Mishra Mahima Signed-off-by: Jonathan Solnit --- drivers/char/diag/diagchar.h | 1 + drivers/char/diag/diagchar_core.c | 1 + drivers/char/diag/diagfwd.c | 44 ++++++++++++++++++++++++++++++--------- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/drivers/char/diag/diagchar.h b/drivers/char/diag/diagchar.h index de3e160762e9f..c1749d3435732 100644 --- a/drivers/char/diag/diagchar.h +++ b/drivers/char/diag/diagchar.h @@ -537,6 +537,7 @@ struct diagchar_dev { unsigned char *buf_feature_mask_update; uint8_t hdlc_disabled; struct mutex hdlc_disable_mutex; + struct mutex hdlc_recovery_mutex; struct timer_list hdlc_reset_timer; struct mutex diag_hdlc_mutex; unsigned char *hdlc_buf; diff --git a/drivers/char/diag/diagchar_core.c b/drivers/char/diag/diagchar_core.c index 590f53c36e511..048e1fd60b805 100644 --- a/drivers/char/diag/diagchar_core.c +++ b/drivers/char/diag/diagchar_core.c @@ -3392,6 +3392,7 @@ static int __init diagchar_init(void) mutex_init(&apps_data_mutex); mutex_init(&driver->msg_mask_lock); mutex_init(&driver->diagfwd_channel_mutex); + mutex_init(&driver->hdlc_recovery_mutex); init_waitqueue_head(&driver->wait_q); INIT_WORK(&(driver->diag_drain_work), diag_drain_work_fn); INIT_WORK(&(driver->update_user_clients), diff --git a/drivers/char/diag/diagfwd.c b/drivers/char/diag/diagfwd.c index 65bbe7cdd8347..60d126acae99a 100644 --- a/drivers/char/diag/diagfwd.c +++ b/drivers/char/diag/diagfwd.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2008-2016, The Linux Foundation. All rights reserved. +/* Copyright (c) 2008-2017, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -1348,7 +1348,9 @@ static void diag_hdlc_start_recovery(unsigned char *buf, int len, if (start_ptr) { /* Discard any partial packet reads */ + mutex_lock(&driver->hdlc_recovery_mutex); driver->incoming_pkt.processing = 0; + mutex_unlock(&driver->hdlc_recovery_mutex); diag_process_non_hdlc_pkt(start_ptr, len - i, info); } } @@ -1362,18 +1364,24 @@ void diag_process_non_hdlc_pkt(unsigned char *buf, int len, const uint32_t header_len = sizeof(struct diag_pkt_frame_t); struct diag_pkt_frame_t *actual_pkt = NULL; unsigned char *data_ptr = NULL; - struct diag_partial_pkt_t *partial_pkt = &driver->incoming_pkt; + struct diag_partial_pkt_t *partial_pkt = NULL; - if (!buf || len <= 0) + mutex_lock(&driver->hdlc_recovery_mutex); + if (!buf || len <= 0) { + mutex_unlock(&driver->hdlc_recovery_mutex); return; - - if (!partial_pkt->processing) + } + partial_pkt = &driver->incoming_pkt; + if (!partial_pkt->processing) { + mutex_unlock(&driver->hdlc_recovery_mutex); goto start; + } if (partial_pkt->remaining > len) { if ((partial_pkt->read_len + len) > partial_pkt->capacity) { pr_err("diag: Invalid length %d, %d received in %s\n", partial_pkt->read_len, len, __func__); + mutex_unlock(&driver->hdlc_recovery_mutex); goto end; } memcpy(partial_pkt->data + partial_pkt->read_len, buf, len); @@ -1387,6 +1395,7 @@ void diag_process_non_hdlc_pkt(unsigned char *buf, int len, pr_err("diag: Invalid length during partial read %d, %d received in %s\n", partial_pkt->read_len, partial_pkt->remaining, __func__); + mutex_unlock(&driver->hdlc_recovery_mutex); goto end; } memcpy(partial_pkt->data + partial_pkt->read_len, buf, @@ -1400,20 +1409,27 @@ void diag_process_non_hdlc_pkt(unsigned char *buf, int len, if (partial_pkt->remaining == 0) { actual_pkt = (struct diag_pkt_frame_t *)(partial_pkt->data); data_ptr = partial_pkt->data + header_len; - if (*(uint8_t *)(data_ptr + actual_pkt->length) != CONTROL_CHAR) + if (*(uint8_t *)(data_ptr + actual_pkt->length) != + CONTROL_CHAR) { + mutex_unlock(&driver->hdlc_recovery_mutex); diag_hdlc_start_recovery(buf, len, info); + mutex_lock(&driver->hdlc_recovery_mutex); + } err = diag_process_apps_pkt(data_ptr, actual_pkt->length, info); if (err) { pr_err("diag: In %s, unable to process incoming data packet, err: %d\n", __func__, err); + mutex_unlock(&driver->hdlc_recovery_mutex); goto end; } partial_pkt->read_len = 0; partial_pkt->total_len = 0; partial_pkt->processing = 0; + mutex_unlock(&driver->hdlc_recovery_mutex); goto start; } + mutex_unlock(&driver->hdlc_recovery_mutex); goto end; start: @@ -1426,14 +1442,14 @@ void diag_process_non_hdlc_pkt(unsigned char *buf, int len, diag_send_error_rsp(buf, len); goto end; } - + mutex_lock(&driver->hdlc_recovery_mutex); if (pkt_len + header_len > partial_pkt->capacity) { pr_err("diag: In %s, incoming data is too large for the request buffer %d\n", __func__, pkt_len); + mutex_unlock(&driver->hdlc_recovery_mutex); diag_hdlc_start_recovery(buf, len, info); break; } - if ((pkt_len + header_len) > (len - read_bytes)) { partial_pkt->read_len = len - read_bytes; partial_pkt->total_len = pkt_len + header_len; @@ -1441,19 +1457,27 @@ void diag_process_non_hdlc_pkt(unsigned char *buf, int len, partial_pkt->read_len; partial_pkt->processing = 1; memcpy(partial_pkt->data, buf, partial_pkt->read_len); + mutex_unlock(&driver->hdlc_recovery_mutex); break; } data_ptr = buf + header_len; - if (*(uint8_t *)(data_ptr + actual_pkt->length) != CONTROL_CHAR) + if (*(uint8_t *)(data_ptr + actual_pkt->length) != + CONTROL_CHAR) { + mutex_unlock(&driver->hdlc_recovery_mutex); diag_hdlc_start_recovery(buf, len, info); + mutex_lock(&driver->hdlc_recovery_mutex); + } else hdlc_reset = 0; err = diag_process_apps_pkt(data_ptr, actual_pkt->length, info); - if (err) + if (err) { + mutex_unlock(&driver->hdlc_recovery_mutex); break; + } read_bytes += header_len + pkt_len + 1; buf += header_len + pkt_len + 1; /* advance to next pkt */ + mutex_unlock(&driver->hdlc_recovery_mutex); } end: return;