Re: ath11k: driver for Qualcomm IEEE 802.11ax devices

Re: ath11k: driver for Qualcomm IEEE 802.11ax devices

[Kalle Valo]

+ ath11k

Colin Ian King <colin.king@canonical.com> writes:

> Hi,
>
> Static analysis with Coverity has detected an issue with the following
> commit:
>
> commit d5c65159f2895379e11ca13f62feabe93278985d
> Author: Kalle Valo <kvalo@codeaurora.org>
> Date:   Sat Nov 23 09:58:40 2019 +0200
>
>     ath11k: driver for Qualcomm IEEE 802.11ax devices
>
> The analysis is as follows:
>
> 4623 static struct sk_buff *
> 4624 ath11k_dp_rx_mon_merg_msdus(struct ath11k *ar,
> 4625                            u32 mac_id, struct sk_buff *head_msdu,
> 4626                            struct sk_buff *last_msdu,
> 4627                            struct ieee80211_rx_status *rxs)
> 4628 {
> 4629        struct sk_buff *msdu, *mpdu_buf, *prev_buf;
> 4630        u32 decap_format, wifi_hdr_len;
> 4631        struct hal_rx_desc *rx_desc;
> 4632        char *hdr_desc;
> 4633        u8 *dest;
> 4634        struct ieee80211_hdr_3addr *wh;
> 4635
>
>     assignment: Assigning: mpdu_buf = NULL.
>
> 4636        mpdu_buf = NULL;
>
> 4718 err_merge_fail:
>      null: At condition mpdu_buf, the value of mpdu_buf must be NULL.
>      dead_error_condition: The condition mpdu_buf cannot be true.
>
> 'Constant' variable guards dead code (DEADCODE)
> dead_error_line: Execution cannot reach the expression decap_format !=
> DP_RX_DECAP_TYPE_RAW inside this statement: if (mpdu_buf && decap_forma....
>
>     Local variable mpdu_buf is assigned only once, to a constant value,
> making it effectively constant throughout its scope. If this is not the
> intent, examine the logic to see if there is a missing assignment that
> would make mpdu_buf not remain constant.
>
> 4719        if (mpdu_buf && decap_format != DP_RX_DECAP_TYPE_RAW) {
> 4720                ath11k_dbg(ar->ab, ATH11K_DBG_DATA,
> 4721                           "err_merge_fail mpdu_buf %pK", mpdu_buf);
> 4722                /* Free the head buffer */
> 4723                dev_kfree_skb_any(mpdu_buf);
> 4724        }
>
> Is this indended?

Yeah, this is suspicious and something is wrong in
ath11k_dp_rx_mon_merg_msdus(). Can someone take a look, please?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k