linux-kernel - Re: [PATCH v2] ath10k: Remove msdu from idr when management pkt send fails

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <875zd88ei8.fsf@kamboji.qca.qualcomm.com>
Date:   Thu, 07 May 2020 09:07:11 +0300
From:   Kalle Valo <kvalo@...eaurora.org>
To:     Rakesh Pillai <pillair@...eaurora.org>
Cc:     ath10k@...ts.infradead.org, linux-wireless@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] ath10k: Remove msdu from idr when management pkt send fails

Rakesh Pillai <pillair@...eaurora.org> writes:

> Currently when the sending of any management pkt
> via wmi command fails, the packet is being unmapped
> freed in the error handling. But the idr entry added,
> which is used to track these packet is not getting removed.
>
> Hence, during unload, in wmi cleanup, all the entries
> in IDR are removed and the corresponding buffer is
> attempted to be freed. This can cause a situation where
> one packet is attempted to be freed twice.
>
> Fix this error by rmeoving the msdu from the idr
> list when the sending of a management packet over
> wmi fails.
>
> Tested HW: WCN3990
> Tested FW: WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
>
> Fixes: 1807da49733e ("ath10k: wmi: add management tx by reference support over wmi")
> Signed-off-by: Rakesh Pillai <pillair@...eaurora.org>

[...]

> --- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
> +++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
> @@ -133,6 +133,7 @@ struct wmi_ops {
>  	struct sk_buff *(*gen_mgmt_tx_send)(struct ath10k *ar,
>  					    struct sk_buff *skb,
>  					    dma_addr_t paddr);
> +	int (*cleanup_mgmt_tx_send)(struct ath10k *ar, struct sk_buff *msdu);
>  	struct sk_buff *(*gen_dbglog_cfg)(struct ath10k *ar, u64 module_enable,
>  					  u32 log_level);
>  	struct sk_buff *(*gen_pktlog_enable)(struct ath10k *ar, u32 filter);
> @@ -442,6 +443,15 @@ ath10k_wmi_get_txbf_conf_scheme(struct ath10k *ar)
>  }
>  
>  static inline int
> +ath10k_wmi_cleanup_mgmt_tx_send(struct ath10k *ar, struct sk_buff *msdu)
> +{
> +	if (!ar->wmi.ops->cleanup_mgmt_tx_send)
> +		return -EOPNOTSUPP;
> +
> +	return ar->wmi.ops->cleanup_mgmt_tx_send(ar, msdu);
> +}
> +
> +static inline int
>  ath10k_wmi_mgmt_tx_send(struct ath10k *ar, struct sk_buff *msdu,
>  			dma_addr_t paddr)
>  {
> @@ -457,8 +467,11 @@ ath10k_wmi_mgmt_tx_send(struct ath10k *ar, struct sk_buff *msdu,
>  
>  	ret = ath10k_wmi_cmd_send(ar, skb,
>  				  ar->wmi.cmd->mgmt_tx_send_cmdid);
> -	if (ret)
> +	if (ret) {
> +		/* remove this msdu from idr tracking */
> +		ath10k_wmi_cleanup_mgmt_tx_send(ar, msdu);
>  		return ret;
> +	}

I missed that this call was in wmi-ops.h, but the idea is that file
should be just a dumb wrapper and not have any logic. So I moved this to
mac.c, the functionality should be the same but please do check my
changes:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=master-pending&id=71195d2244ed812c73dc617f7536566400f7ce87

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches