From 83366dd9ddb9337450f704ceef750a06c69df9ff Mon Sep 17 00:00:00 2001 From: Franky Lin Date: Wed, 5 Jul 2017 18:04:55 -0700 Subject: [PATCH] net: wireless: bcmdhd: add log trace event length check In log trace event parsing routine, add appropriate length check before accessing event data to prevent out of boundary memory access Bug: 37305633 Signed-off-by: Franky Lin Change-Id: I267369957f9b8788f254d9433eb7787b75fb04bc --- drivers/net/wireless/bcmdhd/dhd_debug.c | 52 +++++++++++++++++-------- drivers/net/wireless/bcmdhd/include/event_log.h | 1 + 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/drivers/net/wireless/bcmdhd/dhd_debug.c b/drivers/net/wireless/bcmdhd/dhd_debug.c index 3c45ced1ddc4f..70f4e2df3b752 100644 --- a/drivers/net/wireless/bcmdhd/dhd_debug.c +++ b/drivers/net/wireless/bcmdhd/dhd_debug.c @@ -394,6 +394,12 @@ dhd_dbg_custom_evnt_handler(dhd_pub_t *dhdp, event_log_hdr_t *hdr, uint32 *data) wl_log_id.t = *data; if (wl_log_id.version != DIAG_VERSION) return BCME_VERSION; + /* custom event log should at least contain a wl_event_log_id_ver_t + * header and an arm cycle count + */ + if (hdr->count < 2) + return BCME_BADLEN; + ts_hdr = (void *)data - sizeof(event_log_hdr_t); if (ts_hdr->tag == EVENT_LOG_TAG_TS) { ts_data = (uint32 *)ts_hdr - ts_hdr->count; @@ -624,7 +630,8 @@ dhd_dbg_msgtrace_log_parser(dhd_pub_t *dhdp, void *event_data, msgtrace_hdr_t *hdr; char *data; int id; - uint32 hdrlen = sizeof(event_log_hdr_t); + const uint32 log_hdr_len = sizeof(event_log_hdr_t); + uint32 log_pyld_len; static uint32 seqnum_prev = 0; event_log_hdr_t *log_hdr; bool event_type = FALSE; @@ -632,6 +639,13 @@ dhd_dbg_msgtrace_log_parser(dhd_pub_t *dhdp, void *event_data, dll_t list_head, *cur; loglist_item_t *log_item; + /* log trace event consists of + * msgtrace header + * event log block header + * event log payload + */ + if (datalen <= MSGTRACE_HDRLEN + EVENT_LOG_BLOCK_HDRLEN) + return; hdr = (msgtrace_hdr_t *)event_data; data = (char *)event_data + MSGTRACE_HDRLEN; datalen -= MSGTRACE_HDRLEN; @@ -640,30 +654,36 @@ dhd_dbg_msgtrace_log_parser(dhd_pub_t *dhdp, void *event_data, return; /* XXX: skip the meaningless pktlen/count and timestamp */ - data += 8; - datalen -= 8; + data += EVENT_LOG_BLOCK_HDRLEN; + datalen -= EVENT_LOG_BLOCK_HDRLEN; /* start from the end and walk through the packet */ dll_init(&list_head); - while (datalen > 0) { - log_hdr = (event_log_hdr_t *)(data + datalen - hdrlen); - /* pratially overwritten entries */ - if ((uint32 *)log_hdr - (uint32 *)data < log_hdr->count) - break; - /* end of frame? */ + while (datalen > log_hdr_len) { + log_hdr = (event_log_hdr_t *)(data + datalen - log_hdr_len); + /* skip zero padding at end of frame */ if (log_hdr->tag == EVENT_LOG_TAG_NULL) { - log_hdr--; - datalen -= hdrlen; + datalen -= log_hdr_len; continue; } + + /* Check argument count, any event log should contain at least + * one argument (4 bytes) for arm cycle count and up to 16 + * arguments + */ + if ((log_hdr->count == 0) || (log_hdr->count > MAX_NO_OF_ARG)) + break; + + log_pyld_len = log_hdr->count * DATA_UNIT_FOR_LOG_CNT; + /* log data should not cross event data boundary */ + if (((char *)log_hdr - data) < log_pyld_len) + break; + /* skip 4 bytes time stamp packet */ if (log_hdr->tag == EVENT_LOG_TAG_TS) { - datalen -= log_hdr->count * 4 + hdrlen; - log_hdr -= log_hdr->count + hdrlen / 4; + datalen -= log_pyld_len + log_hdr_len; continue; } - if (log_hdr->count > MAX_NO_OF_ARG) - break; if (!(log_item = MALLOC(dhdp->osh, sizeof(*log_item)))) { DHD_ERROR(("%s allocating log list item failed\n", __FUNCTION__)); @@ -671,7 +691,7 @@ dhd_dbg_msgtrace_log_parser(dhd_pub_t *dhdp, void *event_data, } log_item->hdr = log_hdr; dll_insert(&log_item->list, &list_head); - datalen -= (log_hdr->count * 4 + hdrlen); + datalen -= (log_pyld_len + log_hdr_len); } while (!dll_empty(&list_head)) { diff --git a/drivers/net/wireless/bcmdhd/include/event_log.h b/drivers/net/wireless/bcmdhd/include/event_log.h index 6f0bbc4e40ec1..3964d203d2fb9 100644 --- a/drivers/net/wireless/bcmdhd/include/event_log.h +++ b/drivers/net/wireless/bcmdhd/include/event_log.h @@ -141,6 +141,7 @@ #define LOGSTRS_MAGIC 0x4C4F4753 #define LOGSTRS_VERSION 0x1 +#define EVENT_LOG_BLOCK_HDRLEN 8 /* * There are multiple levels of objects define here: