forked from rpms/kernel
		
	
		
			
				
	
	
		
			165 lines
		
	
	
		
			6.1 KiB
		
	
	
	
		
			Diff
		
	
	
	
	
	
			
		
		
	
	
			165 lines
		
	
	
		
			6.1 KiB
		
	
	
	
		
			Diff
		
	
	
	
	
	
| From patchwork Thu Mar 15 11:31:33 2018
 | |
| Content-Type: text/plain; charset="utf-8"
 | |
| MIME-Version: 1.0
 | |
| Content-Transfer-Encoding: 7bit
 | |
| Subject: wcn36xx: Fix firmware crash due to corrupted buffer address
 | |
| X-Patchwork-Submitter: Ramon Fried <rfried@codeaurora.org>
 | |
| X-Patchwork-Id: 131764
 | |
| Message-Id: <20180315113133.28791-1-rfried@codeaurora.org>
 | |
| To: k.eugene.e@gmail.com, kvalo@codeaurora.org,
 | |
|  wcn36xx@lists.infradead.org, linux-wireless@vger.kernel.org
 | |
| Cc: Loic Poulain <loic.poulain@linaro.org>, Ramon Fried <rfried@codeaurora.org>
 | |
| Date: Thu, 15 Mar 2018 13:31:33 +0200
 | |
| From: Ramon Fried <rfried@codeaurora.org>
 | |
| List-Id: <linux-wireless.vger.kernel.org>
 | |
| 
 | |
| From: Loic Poulain <loic.poulain@linaro.org>
 | |
| 
 | |
| 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: Loic Poulain <loic.poulain@linaro.org>
 | |
| Signed-off-by: Ramon Fried <rfried@codeaurora.org>
 | |
| ---
 | |
|  drivers/net/wireless/ath/wcn36xx/dxe.c  | 13 ++++---------
 | |
|  drivers/net/wireless/ath/wcn36xx/dxe.h  |  3 ++-
 | |
|  drivers/net/wireless/ath/wcn36xx/txrx.c | 32 ++++++++++----------------------
 | |
|  3 files changed, 16 insertions(+), 32 deletions(-)
 | |
| 
 | |
| -- 
 | |
| The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
 | |
| a Linux Foundation Collaborative Project
 | |
| 
 | |
| diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c b/drivers/net/wireless/ath/wcn36xx/dxe.c
 | |
| index 7d5ecaf02288..2c3b899a88fa 100644
 | |
| --- a/drivers/net/wireless/ath/wcn36xx/dxe.c
 | |
| +++ b/drivers/net/wireless/ath/wcn36xx/dxe.c
 | |
| @@ -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;
 | |
|  
 | |
| diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.h b/drivers/net/wireless/ath/wcn36xx/dxe.h
 | |
| index 2bc376c5391b..ce580960d109 100644
 | |
| --- a/drivers/net/wireless/ath/wcn36xx/dxe.h
 | |
| +++ b/drivers/net/wireless/ath/wcn36xx/dxe.h
 | |
| @@ -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_ */
 | |
| diff --git a/drivers/net/wireless/ath/wcn36xx/txrx.c b/drivers/net/wireless/ath/wcn36xx/txrx.c
 | |
| index 22304edc5948..b1768ed6b0be 100644
 | |
| --- a/drivers/net/wireless/ath/wcn36xx/txrx.c
 | |
| +++ b/drivers/net/wireless/ath/wcn36xx/txrx.c
 | |
| @@ -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);
 | |
|  }
 |