Commit e5f99081 authored by Loic Poulain's avatar Loic Poulain Committed by Kalle Valo

wcn36xx: Fix firmware crash due to corrupted buffer address

wcn36xx_start_tx function retrieves the buffer descriptor from the
channel control queue to start filling tx buffer information. However,
nothing prevents this same buffer to be concurrently accessed in a
concurent tx call, leading to potential buffer coruption and firmware
crash (observed during iperf test). The channel control queue should
only be accessed and updated with the channel lock.

Fix this issue by using a local buffer descriptor which will be copied
in the thread-safe wcn36xx_dxe_tx_frame.

Note that buffer descriptor size is few bytes so the introduced copy
overhead is insignificant. Moreover, this allows to keep the locked
section minimal.
Signed-off-by: default avatarLoic Poulain <loic.poulain@linaro.org>
Signed-off-by: default avatarRamon Fried <rfried@codeaurora.org>
Signed-off-by: default avatarKalle Valo <kvalo@codeaurora.org>
parent ee35eecb
......@@ -27,15 +27,6 @@
#include "wcn36xx.h"
#include "txrx.h"
void *wcn36xx_dxe_get_next_bd(struct wcn36xx *wcn, bool is_low)
{
struct wcn36xx_dxe_ch *ch = is_low ?
&wcn->dxe_tx_l_ch :
&wcn->dxe_tx_h_ch;
return ch->head_blk_ctl->bd_cpu_addr;
}
static void wcn36xx_ccu_write_register(struct wcn36xx *wcn, int addr, int data)
{
wcn36xx_dbg(WCN36XX_DBG_DXE,
......@@ -648,6 +639,7 @@ void wcn36xx_dxe_free_mem_pools(struct wcn36xx *wcn)
int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
struct wcn36xx_vif *vif_priv,
struct wcn36xx_tx_bd *bd,
struct sk_buff *skb,
bool is_low)
{
......@@ -681,6 +673,9 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
ctl->skb = NULL;
desc = ctl->desc;
/* write buffer descriptor */
memcpy(ctl->bd_cpu_addr, bd, sizeof(*bd));
/* Set source address of the BD we send */
desc->src_addr_l = ctl->bd_phy_addr;
......
......@@ -452,6 +452,7 @@ struct wcn36xx_dxe_mem_pool {
dma_addr_t phy_addr;
};
struct wcn36xx_tx_bd;
struct wcn36xx_vif;
int wcn36xx_dxe_allocate_mem_pools(struct wcn36xx *wcn);
void wcn36xx_dxe_free_mem_pools(struct wcn36xx *wcn);
......@@ -463,8 +464,8 @@ void wcn36xx_dxe_deinit(struct wcn36xx *wcn);
int wcn36xx_dxe_init_channels(struct wcn36xx *wcn);
int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
struct wcn36xx_vif *vif_priv,
struct wcn36xx_tx_bd *bd,
struct sk_buff *skb,
bool is_low);
void wcn36xx_dxe_tx_ack_ind(struct wcn36xx *wcn, u32 status);
void *wcn36xx_dxe_get_next_bd(struct wcn36xx *wcn, bool is_low);
#endif /* _DXE_H_ */
......@@ -272,21 +272,9 @@ int wcn36xx_start_tx(struct wcn36xx *wcn,
bool is_low = ieee80211_is_data(hdr->frame_control);
bool bcast = is_broadcast_ether_addr(hdr->addr1) ||
is_multicast_ether_addr(hdr->addr1);
struct wcn36xx_tx_bd *bd = wcn36xx_dxe_get_next_bd(wcn, is_low);
if (!bd) {
/*
* TX DXE are used in pairs. One for the BD and one for the
* actual frame. The BD DXE's has a preallocated buffer while
* the skb ones does not. If this isn't true something is really
* wierd. TODO: Recover from this situation
*/
wcn36xx_err("bd address may not be NULL for BD DXE\n");
return -EINVAL;
}
struct wcn36xx_tx_bd bd;
memset(bd, 0, sizeof(*bd));
memset(&bd, 0, sizeof(bd));
wcn36xx_dbg(WCN36XX_DBG_TX,
"tx skb %p len %d fc %04x sn %d %s %s\n",
......@@ -296,10 +284,10 @@ int wcn36xx_start_tx(struct wcn36xx *wcn,
wcn36xx_dbg_dump(WCN36XX_DBG_TX_DUMP, "", skb->data, skb->len);
bd->dpu_rf = WCN36XX_BMU_WQ_TX;
bd.dpu_rf = WCN36XX_BMU_WQ_TX;
bd->tx_comp = !!(info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS);
if (bd->tx_comp) {
bd.tx_comp = !!(info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS);
if (bd.tx_comp) {
wcn36xx_dbg(WCN36XX_DBG_DXE, "TX_ACK status requested\n");
spin_lock_irqsave(&wcn->dxe_lock, flags);
if (wcn->tx_ack_skb) {
......@@ -321,13 +309,13 @@ int wcn36xx_start_tx(struct wcn36xx *wcn,
/* Data frames served first*/
if (is_low)
wcn36xx_set_tx_data(bd, wcn, &vif_priv, sta_priv, skb, bcast);
wcn36xx_set_tx_data(&bd, wcn, &vif_priv, sta_priv, skb, bcast);
else
/* MGMT and CTRL frames are handeld here*/
wcn36xx_set_tx_mgmt(bd, wcn, &vif_priv, skb, bcast);
wcn36xx_set_tx_mgmt(&bd, wcn, &vif_priv, skb, bcast);
buff_to_be((u32 *)bd, sizeof(*bd)/sizeof(u32));
bd->tx_bd_sign = 0xbdbdbdbd;
buff_to_be((u32 *)&bd, sizeof(bd)/sizeof(u32));
bd.tx_bd_sign = 0xbdbdbdbd;
return wcn36xx_dxe_tx_frame(wcn, vif_priv, skb, is_low);
return wcn36xx_dxe_tx_frame(wcn, vif_priv, &bd, skb, is_low);
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment