linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: <yhchuang@realtek.com>
To: <kvalo@codeaurora.org>
Cc: <linux-wireless@vger.kernel.org>, <briannorris@chromium.org>
Subject: [PATCH 08/14] rtw88: fix beaconing mode rsvd_page memory violation issue
Date: Wed, 2 Oct 2019 14:35:25 +0800	[thread overview]
Message-ID: <20191002063531.18135-9-yhchuang@realtek.com> (raw)
In-Reply-To: <20191002063531.18135-1-yhchuang@realtek.com>

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

When downloading the reserved page, the first page always contains
a beacon for the firmware to reference. For non-beaconing modes such
as station mode, also put a blank skb with length=1.

And for the beaconing modes, driver will get a real beacon with a
length approximate to the page size. But as the beacon is always put
at the first page, it does not need a tx_desc, because the TX path
will generate one when TXing the reserved page to the hardware. So we
could allocate a buffer with a size smaller than the reserved page,
when using memcpy() to copy the content of reserved page to the buffer,
the over-sized reserved page will violate the kernel memory.

To fix it, add the tx_desc before memcpy() the reserved packets to
the buffer, then we can get SKBs with correct length when counting
the pages in total. And for page 0, count the extra tx_desc_sz that
the TX path will generate. This way, the first beacon that allocated
without tx_desc can be counted with the extra tx_desc_sz to get
actual pages it requires.

Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/fw.c | 52 ++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index 51649df7cc98..65594393dd1e 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -672,9 +672,6 @@ static void rtw_rsvd_page_list_to_buf(struct rtw_dev *rtwdev, u8 page_size,
 {
 	struct sk_buff *skb = rsvd_pkt->skb;
 
-	if (rsvd_pkt->add_txdesc)
-		rtw_fill_rsvd_page_desc(rtwdev, skb);
-
 	if (page >= 1)
 		memcpy(buf + page_margin + page_size * (page - 1),
 		       skb->data, skb->len);
@@ -799,16 +796,37 @@ static u8 *rtw_build_rsvd_page(struct rtw_dev *rtwdev,
 	list_for_each_entry(rsvd_pkt, &rtwdev->rsvd_page_list, list) {
 		iter = rtw_get_rsvd_page_skb(hw, vif, rsvd_pkt->type);
 		if (!iter) {
-			rtw_err(rtwdev, "fail to build rsvd packet\n");
+			rtw_err(rtwdev, "failed to build rsvd packet\n");
 			goto release_skb;
 		}
+
+		/* Fill the tx_desc for the rsvd pkt that requires one.
+		 * And iter->len will be added with size of tx_desc_sz.
+		 */
+		if (rsvd_pkt->add_txdesc)
+			rtw_fill_rsvd_page_desc(rtwdev, iter);
+
 		rsvd_pkt->skb = iter;
 		rsvd_pkt->page = total_page;
-		if (rsvd_pkt->add_txdesc)
+
+		/* Reserved page is downloaded via TX path, and TX path will
+		 * generate a tx_desc at the header to describe length of
+		 * the buffer. If we are not counting page numbers with the
+		 * size of tx_desc added at the first rsvd_pkt (usually a
+		 * beacon, firmware default refer to the first page as the
+		 * content of beacon), we could generate a buffer which size
+		 * is smaller than the actual size of the whole rsvd_page
+		 */
+		if (total_page == 0) {
+			if (rsvd_pkt->type != RSVD_BEACON) {
+				rtw_err(rtwdev, "first page should be a beacon\n");
+				goto release_skb;
+			}
 			total_page += rtw_len_to_page(iter->len + tx_desc_sz,
 						      page_size);
-		else
+		} else {
 			total_page += rtw_len_to_page(iter->len, page_size);
+		}
 	}
 
 	if (total_page > rtwdev->fifo.rsvd_drv_pg_num) {
@@ -821,13 +839,24 @@ static u8 *rtw_build_rsvd_page(struct rtw_dev *rtwdev,
 	if (!buf)
 		goto release_skb;
 
+	/* Copy the content of each rsvd_pkt to the buf, and they should
+	 * be aligned to the pages.
+	 *
+	 * Note that the first rsvd_pkt is a beacon no matter what vif->type.
+	 * And that rsvd_pkt does not require tx_desc because when it goes
+	 * through TX path, the TX path will generate one for it.
+	 */
 	list_for_each_entry(rsvd_pkt, &rtwdev->rsvd_page_list, list) {
 		rtw_rsvd_page_list_to_buf(rtwdev, page_size, page_margin,
 					  page, buf, rsvd_pkt);
-		page += rtw_len_to_page(rsvd_pkt->skb->len, page_size);
-	}
-	list_for_each_entry(rsvd_pkt, &rtwdev->rsvd_page_list, list)
+		if (page == 0)
+			page += rtw_len_to_page(rsvd_pkt->skb->len +
+						tx_desc_sz, page_size);
+		else
+			page += rtw_len_to_page(rsvd_pkt->skb->len, page_size);
+
 		kfree_skb(rsvd_pkt->skb);
+	}
 
 	return buf;
 
@@ -880,6 +909,11 @@ int rtw_fw_download_rsvd_page(struct rtw_dev *rtwdev, struct ieee80211_vif *vif)
 		goto free;
 	}
 
+	/* The last thing is to download the *ONLY* beacon again, because
+	 * the previous tx_desc is to describe the total rsvd page. Download
+	 * the beacon again to replace the TX desc header, and we will get
+	 * a correct tx_desc for the beacon in the rsvd page.
+	 */
 	ret = rtw_download_beacon(rtwdev, vif);
 	if (ret) {
 		rtw_err(rtwdev, "failed to download beacon\n");
-- 
2.17.1


  parent reply	other threads:[~2019-10-02  6:35 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-02  6:35 [PATCH 00/14] rtw88: add software TX queue support yhchuang
2019-10-02  6:35 ` [PATCH 01/14] rtw88: check firmware leave lps successfully yhchuang
2019-10-04 13:45   ` Kalle Valo
2019-10-02  6:35 ` [PATCH 02/14] rtw88: allows to set RTS in TX descriptor yhchuang
2019-10-02  6:35 ` [PATCH 03/14] rtw88: add driver TX queue support yhchuang
2019-10-02  6:35 ` [PATCH 04/14] rtw88: take over rate control from mac80211 yhchuang
2019-10-02  6:35 ` [PATCH 05/14] rtw88: report tx rate to mac80211 stack yhchuang
2019-10-02  6:35 ` [PATCH 06/14] rtw88: add TX-AMSDU support yhchuang
2019-10-02  6:35 ` [PATCH 07/14] rtw88: flush hardware tx queues yhchuang
2019-10-02  6:35 ` yhchuang [this message]
2019-10-02  6:35 ` [PATCH 09/14] rtw88: Don't set RX_FLAG_DECRYPTED if packet has no encryption yhchuang
2019-10-02  6:35 ` [PATCH 10/14] rtw88: configure TX queue EDCA parameters yhchuang
2019-10-02  6:35 ` [PATCH 11/14] rtw88: raise firmware version debug level yhchuang
2019-10-02  6:35 ` [PATCH 12/14] rtw88: use struct rtw_fw_hdr to access firmware header yhchuang
2019-10-02  6:35 ` [PATCH 13/14] rtw88: fix NSS of hw_cap yhchuang
2019-10-02  6:35 ` [PATCH 14/14] rtw88: fix error handling when setup efuse info yhchuang

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=20191002063531.18135-9-yhchuang@realtek.com \
    --to=yhchuang@realtek.com \
    --cc=briannorris@chromium.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).