On 11/16/2022 3:58 PM, Minsuk Kang wrote:
This patch fixes a slab-out-of-bounds read in brcmfmac that occurs in
cfg80211_find_elem_match() called from brcmf_inform_single_bss() when
the offset and length values of information elements provided by the
device exceed the boundary of the escan buffer that contains information
elements. The patch adds a check that makes the function return -EINVAL
if that is the case. Note that the negative return is handled by the
caller, brcmf_inform_bss().
[...]
Reviewed-by: Arend van Spriel <arend.vanspriel@xxxxxxxxxxxx>
Reported-by: Dokyung Song <dokyungs@xxxxxxxxxxxx>
Reported-by: Jisoo Jang <jisoo.jang@xxxxxxxxxxxx>
Reported-by: Minsuk Kang <linuxlovemin@xxxxxxxxxxxx>
Reported-by: kernel test robot <lkp@xxxxxxxxx>
Signed-off-by: Minsuk Kang <linuxlovemin@xxxxxxxxxxxx>
---
v1->v2: Use the correct format for size_t in bphy_err()
.../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index ae9507dec74a..2148027eb42b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3298,6 +3298,13 @@ static s32 brcmf_inform_single_bss(struct brcmf_cfg80211_info *cfg,
notify_ielen = le32_to_cpu(bi->ie_length);
bss_data.signal = (s16)le16_to_cpu(bi->RSSI) * 100;
+ if ((unsigned long)notify_ie + notify_ielen -
+ (unsigned long)cfg->escan_info.escan_buf > BRCMF_ESCAN_BUF_SIZE) {
+ bphy_err(drvr, "Invalid information element offset: %u, length: %zu\n",
+ le16_to_cpu(bi->ie_offset), notify_ielen);
+ return -EINVAL;
+ }
+
Maybe this works, but it was not immediately obvious to me. Also this
seems late in processing the scan results. Better catch it early and
check the ie_offset and ie_length values in
brcmf_cfg80211_escan_handler() when processing the partial result event.
It already checks bi->length there so add a check there:
bss_ie_offset = le16_to_cpu(bi->ie_offset);
bss_ie_length = le16_to_cpu(bi->ie_length);
if (bi->ie_offset + bi->ie_length > bi->length) {
bphy_err(drvr, "Ignoring invalid information element offset: %u,
length: %zu\n"
bss_ie_offset, bss_ie_length);
goto exit;
}
Regards,
Arend
