From f7248d774e2e1867593978a13dd805e0de11bc93 Mon Sep 17 00:00:00 2001 From: Srinivas Girigowda Date: Wed, 2 Dec 2020 17:50:19 -0800 Subject: [PATCH] qcacmn: Fix possible OOB read in cnss_diag_cmd_handler The nla_data coming from user space is a variable length data, but the driver is checking nla_len() against a fixed length struct only. It is possible that the nla_len() check against fixed length struct may pass and if the nla_data does not have the payload and it may result in possible out-of-bound read (slot->payload). Hence the fix is to, check if nla_len() is atleast more than the fixed length struct and also account for payload length. Change-Id: I2e68d55c0411cff55908c1704031e3c070f3316e CRs-Fixed: 2825763 --- utils/fwlog/dbglog_host.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/utils/fwlog/dbglog_host.c b/utils/fwlog/dbglog_host.c index 8c97555371..b2d2582a10 100644 --- a/utils/fwlog/dbglog_host.c +++ b/utils/fwlog/dbglog_host.c @@ -4224,6 +4224,7 @@ static void cnss_diag_cmd_handler(const void *data, int data_len, { struct dbglog_slot *slot = NULL; struct nlattr *tb[QCA_WLAN_VENDOR_ATTR_MAX + 1]; + int len; /* * audit note: it is ok to pass a NULL policy here since a @@ -4242,15 +4243,17 @@ static void cnss_diag_cmd_handler(const void *data, int data_len, return; } - if (nla_len(tb[CLD80211_ATTR_DATA]) != sizeof(struct dbglog_slot)) { - AR_DEBUG_PRINTF(ATH_DEBUG_ERR, ("%s: attr length check fails\n", + len = nla_len(tb[CLD80211_ATTR_DATA]); + if (len < sizeof(struct dbglog_slot)) { + AR_DEBUG_PRINTF(ATH_DEBUG_ERR, ("%s: attr length less than sizeof(struct dbglog_slot)\n", __func__)); return; } - slot = (struct dbglog_slot *)nla_data(tb[CLD80211_ATTR_DATA]); - if (!slot) { - AR_DEBUG_PRINTF(ATH_DEBUG_ERR, ("%s: data NULL\n", __func__)); + slot = (struct dbglog_slot *)nla_data(tb[CLD80211_ATTR_DATA]); + if (len != (sizeof(struct dbglog_slot) + (uint64_t) slot->length)) { + AR_DEBUG_PRINTF(ATH_DEBUG_ERR, ("%s: attr length check fails\n", + __func__)); return; }