linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
To: kvalo@codeaurora.org, linux-wireless@vger.kernel.org,
	wcn36xx@lists.infradead.org
Cc: loic.poulain@linaro.org, benl@squareup.com,
	daniel.thompson@linaro.org, bryan.odonoghue@linaro.org
Subject: [PATCH 4/4] wcn36xx: Put DXE block into reset before freeing memory
Date: Fri, 15 Oct 2021 14:17:41 +0100	[thread overview]
Message-ID: <20211015131741.2455824-5-bryan.odonoghue@linaro.org> (raw)
In-Reply-To: <20211015131741.2455824-1-bryan.odonoghue@linaro.org>

When deiniting the DXE hardware we should reset the block to ensure there
is no spurious DMA write transaction from the downstream WCNSS to upstream
MSM at a skbuff address we will have released.

This is actually a pretty serious bug. Immediately after the reset we
release skbs, skbs which are from the perspective of the WCNSS DXE still
valid addresses for DMA.

Without first placing the DXE block into reset, it is possible for an
upstream DMA transaction to write to skbs we have freed.

We have seen some backtraces from usage in testing on 50k+ devices which
indicates an invalid RX of an APs beacon to unmapped memory.

The logical conclusion is that an RX transaction happened to a region of
memory that was previously valid but was subsequently released.

The only time such a window of opportunity exists is when we have
deallocated the skbs attached to the DMA BDs in other words after doing
wcn36xx_stop().

If we free the skbs on the DMA channel, we need to make sure we have
quiesced potential DMA on that channel prior to freeing.

This patch should eliminate that error.

Fixes: 8e84c2582169 ("wcn36xx: mac80211 driver for Qualcomm WCN3660/WCN3680 hardware")
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/dxe.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c b/drivers/net/wireless/ath/wcn36xx/dxe.c
index e89002502869a..56f605c23f36c 100644
--- a/drivers/net/wireless/ath/wcn36xx/dxe.c
+++ b/drivers/net/wireless/ath/wcn36xx/dxe.c
@@ -1020,6 +1020,8 @@ int wcn36xx_dxe_init(struct wcn36xx *wcn)
 
 void wcn36xx_dxe_deinit(struct wcn36xx *wcn)
 {
+	int reg_data = 0;
+
 	/* Disable channel interrupts */
 	wcn36xx_dxe_disable_ch_int(wcn, WCN36XX_INT_MASK_CHAN_RX_H);
 	wcn36xx_dxe_disable_ch_int(wcn, WCN36XX_INT_MASK_CHAN_RX_L);
@@ -1035,6 +1037,10 @@ void wcn36xx_dxe_deinit(struct wcn36xx *wcn)
 		wcn->tx_ack_skb = NULL;
 	}
 
+	/* Put the DXE block into reset before freeing memory */
+	reg_data = WCN36XX_DXE_REG_RESET;
+	wcn36xx_dxe_write_register(wcn, WCN36XX_DXE_REG_CSR_RESET, reg_data);
+
 	wcn36xx_dxe_ch_free_skbs(wcn, &wcn->dxe_rx_l_ch);
 	wcn36xx_dxe_ch_free_skbs(wcn, &wcn->dxe_rx_h_ch);
 
-- 
2.33.0


  parent reply	other threads:[~2021-10-15 13:15 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-15 13:17 [PATCH 0/4] wcn36xx: Fix DMA buffer allocation and free logic Bryan O'Donoghue
2021-10-15 13:17 ` [PATCH 1/4] wcn36xx: Fix DXE lock layering violation Bryan O'Donoghue
2021-10-15 13:17 ` [PATCH 2/4] wcn36xx: Fix DXE/DMA channel enable/disable cycle Bryan O'Donoghue
2021-10-15 13:17 ` [PATCH 3/4] wcn36xx: Release DMA channel descriptor allocations Bryan O'Donoghue
2021-10-15 13:17 ` Bryan O'Donoghue [this message]
2021-10-17  1:28   ` [PATCH 4/4] wcn36xx: Put DXE block into reset before freeing memory Bryan O'Donoghue

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=20211015131741.2455824-5-bryan.odonoghue@linaro.org \
    --to=bryan.odonoghue@linaro.org \
    --cc=benl@squareup.com \
    --cc=daniel.thompson@linaro.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=loic.poulain@linaro.org \
    --cc=wcn36xx@lists.infradead.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).