From: Sven Eckelmann <sven@narfation.org> To: ath11k@lists.infradead.org Cc: linux-wireless@vger.kernel.org, Wen Gong <quic_wgong@quicinc.com>, Sven Eckelmann <sven@narfation.org>, stable@vger.kernel.org Subject: [PATCH] ath11k: Fix buffer overflow when scanning with extraie Date: Tue, 7 Dec 2021 15:29:13 +0100 [thread overview] Message-ID: <20211207142913.1734635-1-sven@narfation.org> (raw) If cfg80211 is providing extraie's for a scanning process then ath11k will copy that over to the firmware. The extraie.len is a 32 bit value in struct element_info and describes the amount of bytes for the vendor information elements. The WMI_TLV packet is having a special WMI_TAG_ARRAY_BYTE section. This section can have a (payload) length up to 65535 bytes because the WMI_TLV_LEN can store up to 16 bits. The code was missing such a check and could have created a scan request which cannot be parsed correctly by the firmware. But the bigger problem was the allocation of the buffer. It has to align the TLV sections by 4 bytes. But the code was using an u8 to store the newly calculated length of this section (with alignment). And the new calculated length was then used to allocate the skbuff. But the actual code to copy in the data is using the extraie.len and not the calculated "aligned" length. The length of extraie with IEEE80211_HW_SINGLE_SCAN_ON_ALL_BANDS enabled was 264 bytes during tests with a QCA Milan card. But it only allocated 8 bytes (264 bytes % 256) for it. As consequence, the code to memcpy the extraie into the skb was then just overwriting data after skb->end. Things like shinfo were therefore corrupted. This could usually be seen by a crash in skb_zcopy_clear which tried to call a ubuf_info callback (using a bogus address). Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-02892.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1 Cc: stable@vger.kernel.org Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices") Signed-off-by: Sven Eckelmann <sven@narfation.org> --- drivers/net/wireless/ath/ath11k/wmi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/ath/ath11k/wmi.c b/drivers/net/wireless/ath/ath11k/wmi.c index 6da16d095e37..d817ea468ab5 100644 --- a/drivers/net/wireless/ath/ath11k/wmi.c +++ b/drivers/net/wireless/ath/ath11k/wmi.c @@ -2115,7 +2115,7 @@ int ath11k_wmi_send_scan_start_cmd(struct ath11k *ar, void *ptr; int i, ret, len; u32 *tmp_ptr; - u8 extraie_len_with_pad = 0; + u16 extraie_len_with_pad = 0; struct hint_short_ssid *s_ssid = NULL; struct hint_bssid *hint_bssid = NULL; @@ -2134,7 +2134,7 @@ int ath11k_wmi_send_scan_start_cmd(struct ath11k *ar, len += sizeof(*bssid) * params->num_bssid; len += TLV_HDR_SIZE; - if (params->extraie.len) + if (params->extraie.len && params->extraie.len <= 0xFFFF) extraie_len_with_pad = roundup(params->extraie.len, sizeof(u32)); len += extraie_len_with_pad; @@ -2241,7 +2241,7 @@ int ath11k_wmi_send_scan_start_cmd(struct ath11k *ar, FIELD_PREP(WMI_TLV_LEN, len); ptr += TLV_HDR_SIZE; - if (params->extraie.len) + if (extraie_len_with_pad) memcpy(ptr, params->extraie.ptr, params->extraie.len); -- 2.30.2
WARNING: multiple messages have this Message-ID (diff)
From: Sven Eckelmann <sven@narfation.org> To: ath11k@lists.infradead.org Cc: linux-wireless@vger.kernel.org, Wen Gong <quic_wgong@quicinc.com>, Sven Eckelmann <sven@narfation.org>, stable@vger.kernel.org Subject: [PATCH] ath11k: Fix buffer overflow when scanning with extraie Date: Tue, 7 Dec 2021 15:29:13 +0100 [thread overview] Message-ID: <20211207142913.1734635-1-sven@narfation.org> (raw) If cfg80211 is providing extraie's for a scanning process then ath11k will copy that over to the firmware. The extraie.len is a 32 bit value in struct element_info and describes the amount of bytes for the vendor information elements. The WMI_TLV packet is having a special WMI_TAG_ARRAY_BYTE section. This section can have a (payload) length up to 65535 bytes because the WMI_TLV_LEN can store up to 16 bits. The code was missing such a check and could have created a scan request which cannot be parsed correctly by the firmware. But the bigger problem was the allocation of the buffer. It has to align the TLV sections by 4 bytes. But the code was using an u8 to store the newly calculated length of this section (with alignment). And the new calculated length was then used to allocate the skbuff. But the actual code to copy in the data is using the extraie.len and not the calculated "aligned" length. The length of extraie with IEEE80211_HW_SINGLE_SCAN_ON_ALL_BANDS enabled was 264 bytes during tests with a QCA Milan card. But it only allocated 8 bytes (264 bytes % 256) for it. As consequence, the code to memcpy the extraie into the skb was then just overwriting data after skb->end. Things like shinfo were therefore corrupted. This could usually be seen by a crash in skb_zcopy_clear which tried to call a ubuf_info callback (using a bogus address). Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-02892.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1 Cc: stable@vger.kernel.org Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices") Signed-off-by: Sven Eckelmann <sven@narfation.org> --- drivers/net/wireless/ath/ath11k/wmi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/ath/ath11k/wmi.c b/drivers/net/wireless/ath/ath11k/wmi.c index 6da16d095e37..d817ea468ab5 100644 --- a/drivers/net/wireless/ath/ath11k/wmi.c +++ b/drivers/net/wireless/ath/ath11k/wmi.c @@ -2115,7 +2115,7 @@ int ath11k_wmi_send_scan_start_cmd(struct ath11k *ar, void *ptr; int i, ret, len; u32 *tmp_ptr; - u8 extraie_len_with_pad = 0; + u16 extraie_len_with_pad = 0; struct hint_short_ssid *s_ssid = NULL; struct hint_bssid *hint_bssid = NULL; @@ -2134,7 +2134,7 @@ int ath11k_wmi_send_scan_start_cmd(struct ath11k *ar, len += sizeof(*bssid) * params->num_bssid; len += TLV_HDR_SIZE; - if (params->extraie.len) + if (params->extraie.len && params->extraie.len <= 0xFFFF) extraie_len_with_pad = roundup(params->extraie.len, sizeof(u32)); len += extraie_len_with_pad; @@ -2241,7 +2241,7 @@ int ath11k_wmi_send_scan_start_cmd(struct ath11k *ar, FIELD_PREP(WMI_TLV_LEN, len); ptr += TLV_HDR_SIZE; - if (params->extraie.len) + if (extraie_len_with_pad) memcpy(ptr, params->extraie.ptr, params->extraie.len); -- 2.30.2 -- ath11k mailing list ath11k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath11k
next reply other threads:[~2021-12-07 14:30 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-12-07 14:29 Sven Eckelmann [this message] 2021-12-07 14:29 ` [PATCH] ath11k: Fix buffer overflow when scanning with extraie Sven Eckelmann 2021-12-09 7:59 ` Kalle Valo 2021-12-09 7:59 ` Kalle Valo
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20211207142913.1734635-1-sven@narfation.org \ --to=sven@narfation.org \ --cc=ath11k@lists.infradead.org \ --cc=linux-wireless@vger.kernel.org \ --cc=quic_wgong@quicinc.com \ --cc=stable@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.