| From 9177f2be21b9606417deefc9e806f21e459efa17 Mon Sep 17 00:00:00 2001 |
| From: Dan Carpenter <dan.carpenter@oracle.com> |
| Date: Wed, 24 Apr 2019 12:52:18 +0300 |
| Subject: brcm80211: potential NULL dereference in |
| brcmf_cfg80211_vndr_cmds_dcmd_handler() |
| |
| [ Upstream commit e025da3d7aa4770bb1d1b3b0aa7cc4da1744852d ] |
| |
| If "ret_len" is negative then it could lead to a NULL dereference. |
| |
| The "ret_len" value comes from nl80211_vendor_cmd(), if it's negative |
| then we don't allocate the "dcmd_buf" buffer. Then we pass "ret_len" to |
| brcmf_fil_cmd_data_set() where it is cast to a very high u32 value. |
| Most of the functions in that call tree check whether the buffer we pass |
| is NULL but there are at least a couple places which don't such as |
| brcmf_dbg_hex_dump() and brcmf_msgbuf_query_dcmd(). We memcpy() to and |
| from the buffer so it would result in a NULL dereference. |
| |
| The fix is to change the types so that "ret_len" can't be negative. (If |
| we memcpy() zero bytes to NULL, that's a no-op and doesn't cause an |
| issue). |
| |
| Fixes: 1bacb0487d0e ("brcmfmac: replace cfg80211 testmode with vendor command") |
| Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> |
| Signed-off-by: Kalle Valo <kvalo@codeaurora.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c | 5 +++-- |
| 1 file changed, 3 insertions(+), 2 deletions(-) |
| |
| diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c |
| index 8eff2753abade..d493021f60318 100644 |
| --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c |
| +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c |
| @@ -35,9 +35,10 @@ static int brcmf_cfg80211_vndr_cmds_dcmd_handler(struct wiphy *wiphy, |
| struct brcmf_if *ifp; |
| const struct brcmf_vndr_dcmd_hdr *cmdhdr = data; |
| struct sk_buff *reply; |
| - int ret, payload, ret_len; |
| + unsigned int payload, ret_len; |
| void *dcmd_buf = NULL, *wr_pointer; |
| u16 msglen, maxmsglen = PAGE_SIZE - 0x100; |
| + int ret; |
| |
| if (len < sizeof(*cmdhdr)) { |
| brcmf_err("vendor command too short: %d\n", len); |
| @@ -65,7 +66,7 @@ static int brcmf_cfg80211_vndr_cmds_dcmd_handler(struct wiphy *wiphy, |
| brcmf_err("oversize return buffer %d\n", ret_len); |
| ret_len = BRCMF_DCMD_MAXLEN; |
| } |
| - payload = max(ret_len, len) + 1; |
| + payload = max_t(unsigned int, ret_len, len) + 1; |
| dcmd_buf = vzalloc(payload); |
| if (NULL == dcmd_buf) |
| return -ENOMEM; |
| -- |
| 2.20.1 |
| |
