* [bug report] wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices
@ 2023-02-16 9:57 Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2023-02-16 9:57 UTC (permalink / raw)
To: quic_kvalo; +Cc: ath12k
Hello Kalle Valo,
The patch d889913205cf: "wifi: ath12k: driver for Qualcomm Wi-Fi 7
devices" from Nov 28, 2022, leads to the following Smatch static
checker warning:
drivers/net/wireless/ath/ath12k/dp_rx.c:3254 ath12k_dp_rx_frag_h_mpdu() warn: missing error code here? 'ath12k_peer_find_by_id()' failed. 'ret' = '0'
drivers/net/wireless/ath/ath12k/dp_rx.c:3260 ath12k_dp_rx_frag_h_mpdu() warn: missing error code here? 'ath12k_dp_rx_h_defrag()' failed. 'ret' = '0'
drivers/net/wireless/ath/ath12k/dp_rx.c:3266 ath12k_dp_rx_frag_h_mpdu() warn: missing error code here? 'ath12k_dp_rx_h_defrag_reo_reinject()' failed. 'ret' = '0'
drivers/net/wireless/ath/ath12k/dp_rx.c
3161 static int ath12k_dp_rx_frag_h_mpdu(struct ath12k *ar,
3162 struct sk_buff *msdu,
3163 struct hal_reo_dest_ring *ring_desc)
3164 {
3165 struct ath12k_base *ab = ar->ab;
3166 struct hal_rx_desc *rx_desc;
3167 struct ath12k_peer *peer;
3168 struct ath12k_dp_rx_tid *rx_tid;
3169 struct sk_buff *defrag_skb = NULL;
3170 u32 peer_id;
3171 u16 seqno, frag_no;
3172 u8 tid;
3173 int ret = 0;
3174 bool more_frags;
3175
3176 rx_desc = (struct hal_rx_desc *)msdu->data;
3177 peer_id = ath12k_dp_rx_h_peer_id(ab, rx_desc);
3178 tid = ath12k_dp_rx_h_tid(ab, rx_desc);
3179 seqno = ath12k_dp_rx_h_seq_no(ab, rx_desc);
3180 frag_no = ath12k_dp_rx_h_frag_no(ab, msdu);
3181 more_frags = ath12k_dp_rx_h_more_frags(ab, msdu);
3182
3183 if (!ath12k_dp_rx_h_seq_ctrl_valid(ab, rx_desc) ||
3184 !ath12k_dp_rx_h_fc_valid(ab, rx_desc) ||
3185 tid > IEEE80211_NUM_TIDS)
3186 return -EINVAL;
3187
3188 /* received unfragmented packet in reo
3189 * exception ring, this shouldn't happen
3190 * as these packets typically come from
3191 * reo2sw srngs.
3192 */
3193 if (WARN_ON_ONCE(!frag_no && !more_frags))
3194 return -EINVAL;
3195
3196 spin_lock_bh(&ab->base_lock);
3197 peer = ath12k_peer_find_by_id(ab, peer_id);
3198 if (!peer) {
3199 ath12k_warn(ab, "failed to find the peer to de-fragment received fragment peer_id %d\n",
3200 peer_id);
3201 ret = -ENOENT;
3202 goto out_unlock;
3203 }
3204 rx_tid = &peer->rx_tid[tid];
3205
3206 if ((!skb_queue_empty(&rx_tid->rx_frags) && seqno != rx_tid->cur_sn) ||
3207 skb_queue_empty(&rx_tid->rx_frags)) {
3208 /* Flush stored fragments and start a new sequence */
3209 ath12k_dp_rx_frags_cleanup(rx_tid, true);
3210 rx_tid->cur_sn = seqno;
3211 }
3212
3213 if (rx_tid->rx_frag_bitmap & BIT(frag_no)) {
3214 /* Fragment already present */
3215 ret = -EINVAL;
3216 goto out_unlock;
3217 }
3218
3219 if (frag_no > __fls(rx_tid->rx_frag_bitmap))
3220 __skb_queue_tail(&rx_tid->rx_frags, msdu);
3221 else
3222 ath12k_dp_rx_h_sort_frags(ab, &rx_tid->rx_frags, msdu);
3223
3224 rx_tid->rx_frag_bitmap |= BIT(frag_no);
3225 if (!more_frags)
3226 rx_tid->last_frag_no = frag_no;
3227
3228 if (frag_no == 0) {
3229 rx_tid->dst_ring_desc = kmemdup(ring_desc,
3230 sizeof(*rx_tid->dst_ring_desc),
3231 GFP_ATOMIC);
3232 if (!rx_tid->dst_ring_desc) {
3233 ret = -ENOMEM;
3234 goto out_unlock;
3235 }
3236 } else {
3237 ath12k_dp_rx_link_desc_return(ab, ring_desc,
3238 HAL_WBM_REL_BM_ACT_PUT_IN_IDLE);
3239 }
3240
3241 if (!rx_tid->last_frag_no ||
3242 rx_tid->rx_frag_bitmap != GENMASK(rx_tid->last_frag_no, 0)) {
3243 mod_timer(&rx_tid->frag_timer, jiffies +
3244 ATH12K_DP_RX_FRAGMENT_TIMEOUT_MS);
error code?
3245 goto out_unlock;
3246 }
3247
3248 spin_unlock_bh(&ab->base_lock);
3249 del_timer_sync(&rx_tid->frag_timer);
3250 spin_lock_bh(&ab->base_lock);
3251
3252 peer = ath12k_peer_find_by_id(ab, peer_id);
3253 if (!peer)
--> 3254 goto err_frags_cleanup;
here?
3255
3256 if (!ath12k_dp_rx_h_defrag_validate_incr_pn(ar, rx_tid))
3257 goto err_frags_cleanup;
This definitely looks like an error path. Similar warning
in ath12k_reg_chan_list_event().
drivers/net/wireless/ath/ath12k/wmi.c:5195 ath12k_reg_chan_list_event() warn: missing error code here? 'ath12k_reg_build_regd()' failed. 'ret' = '0'
3258
3259 if (ath12k_dp_rx_h_defrag(ar, peer, rx_tid, &defrag_skb))
3260 goto err_frags_cleanup;
3261
3262 if (!defrag_skb)
3263 goto err_frags_cleanup;
3264
3265 if (ath12k_dp_rx_h_defrag_reo_reinject(ar, rx_tid, defrag_skb))
3266 goto err_frags_cleanup;
3267
3268 ath12k_dp_rx_frags_cleanup(rx_tid, false);
3269 goto out_unlock;
3270
3271 err_frags_cleanup:
3272 dev_kfree_skb_any(defrag_skb);
3273 ath12k_dp_rx_frags_cleanup(rx_tid, true);
3274 out_unlock:
3275 spin_unlock_bh(&ab->base_lock);
3276 return ret;
3277 }
regards,
dan carpenter
--
ath12k mailing list
ath12k@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/ath12k
^ permalink raw reply [flat|nested] 5+ messages in thread
* [bug report] wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices
@ 2023-02-16 11:44 Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2023-02-16 11:44 UTC (permalink / raw)
To: quic_kvalo; +Cc: ath12k
Hello Kalle Valo,
The patch d889913205cf: "wifi: ath12k: driver for Qualcomm Wi-Fi 7
devices" from Nov 28, 2022, leads to the following Smatch static
checker warning:
drivers/net/wireless/ath/ath12k/mac.c:1658 ath12k_peer_assoc_h_he()
warn: mask and shift to zero
drivers/net/wireless/ath/ath12k/mac.c
1642 /* the top most byte is used to indicate BSS color info */
1643 arg->peer_he_ops &= 0xffffff;
1644
1645 /* As per section 26.6.1 IEEE Std 802.11ax‐2022, if the Max AMPDU
1646 * Exponent Extension in HE cap is zero, use the arg->peer_max_mpdu
1647 * as calculated while parsing VHT caps(if VHT caps is present)
1648 * or HT caps (if VHT caps is not present).
1649 *
1650 * For non-zero value of Max AMPDU Exponent Extension in HE MAC caps,
1651 * if a HE STA sends VHT cap and HE cap IE in assoc request then, use
1652 * MAX_AMPDU_LEN_FACTOR as 20 to calculate max_ampdu length.
1653 * If a HE STA that does not send VHT cap, but HE and HT cap in assoc
1654 * request, then use MAX_AMPDU_LEN_FACTOR as 16 to calculate max_ampdu
1655 * length.
1656 */
1657 ampdu_factor = (he_cap->he_cap_elem.mac_cap_info[3] &
--> 1658 IEEE80211_HE_MAC_CAP3_MAX_AMPDU_LEN_EXP_MASK) >>
1659 IEEE80211_HE_MAC_CAP3_MAX_AMPDU_LEN_EXP_MASK;
^^^^^
This likely should be shifting by a shift define instead of a _MASK
value.
1660
1661 if (ampdu_factor) {
^^^^^^^^^^^^
Never going to be true.
1662 if (sta->deflink.vht_cap.vht_supported)
1663 arg->peer_max_mpdu = (1 << (IEEE80211_HE_VHT_MAX_AMPDU_FACTOR +
1664 ampdu_factor)) - 1;
1665 else if (sta->deflink.ht_cap.ht_supported)
1666 arg->peer_max_mpdu = (1 << (IEEE80211_HE_HT_MAX_AMPDU_FACTOR +
1667 ampdu_factor)) - 1;
1668 }
1669
regards,
dan carpenter
--
ath12k mailing list
ath12k@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/ath12k
^ permalink raw reply [flat|nested] 5+ messages in thread
* [bug report] wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices
@ 2023-02-16 11:54 Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2023-02-16 11:54 UTC (permalink / raw)
To: quic_kvalo; +Cc: ath12k
Hello Kalle Valo,
The patch d889913205cf: "wifi: ath12k: driver for Qualcomm Wi-Fi 7
devices" from Nov 28, 2022, leads to the following Smatch static
checker warning:
drivers/net/wireless/ath/ath12k/mac.c:4737 ath12k_mac_get_vdev_stats_id()
warn: always true condition '(vdev_stats_id <= 255) => (0-255 <= 255)'
drivers/net/wireless/ath/ath12k/mac.c
4728 static u8
4729 ath12k_mac_get_vdev_stats_id(struct ath12k_vif *arvif)
4730 {
4731 struct ath12k_base *ab = arvif->ar->ab;
4732 u8 vdev_stats_id = 0;
4733
4734 do {
4735 if (ab->free_vdev_stats_id_map & (1LL << vdev_stats_id))
Shifting by more than 63 is undefined.
4736 vdev_stats_id++;
--> 4737 if (vdev_stats_id <= ATH12K_INVAL_VDEV_STATS_ID) {
This if statement seems reversed. It likely should be:
if (vdev_stats_id > ATH12K_INVAL_VDEV_STATS_ID) {
But here as well ATH12K_INVAL_VDEV_STATS_ID is 255 and that's obviously
higher than 63.
4738 vdev_stats_id = ATH12K_INVAL_VDEV_STATS_ID;
4739 break;
4740 }
4741 } else {
4742 ab->free_vdev_stats_id_map |= (1LL << vdev_stats_id);
4743 break;
4744 }
4745 } while (vdev_stats_id);
4746
4747 arvif->vdev_stats_id = vdev_stats_id;
4748 return vdev_stats_id;
4749 }
regards,
dan carpenter
--
ath12k mailing list
ath12k@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/ath12k
^ permalink raw reply [flat|nested] 5+ messages in thread
* [bug report] wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices
@ 2023-02-16 12:28 Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2023-02-16 12:28 UTC (permalink / raw)
To: quic_kvalo; +Cc: ath12k
Hello Kalle Valo,
The patch d889913205cf: "wifi: ath12k: driver for Qualcomm Wi-Fi 7
devices" from Nov 28, 2022, leads to the following Smatch static
checker warning:
drivers/net/wireless/ath/ath12k/mac.c:5872 ath12k_mac_op_unassign_vif_chanctx()
error: double unlocked '&ar->conf_mutex' (orig line 5854)
drivers/net/wireless/ath/ath12k/mac.c
5822 static void
5823 ath12k_mac_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
5824 struct ieee80211_vif *vif,
5825 struct ieee80211_bss_conf *link_conf,
5826 struct ieee80211_chanctx_conf *ctx)
5827 {
5828 struct ath12k *ar = hw->priv;
5829 struct ath12k_base *ab = ar->ab;
5830 struct ath12k_vif *arvif = (void *)vif->drv_priv;
5831 int ret;
5832
5833 mutex_lock(&ar->conf_mutex);
5834
5835 ath12k_dbg(ab, ATH12K_DBG_MAC,
5836 "mac chanctx unassign ptr %pK vdev_id %i\n",
5837 ctx, arvif->vdev_id);
5838
5839 WARN_ON(!arvif->is_started);
5840
5841 if (ab->hw_params->vdev_start_delay &&
5842 arvif->vdev_type == WMI_VDEV_TYPE_MONITOR &&
5843 ath12k_peer_find_by_addr(ab, ar->mac_addr))
5844 ath12k_peer_delete(ar, arvif->vdev_id, ar->mac_addr);
5845
5846 if (arvif->vdev_type == WMI_VDEV_TYPE_MONITOR) {
5847 ret = ath12k_mac_monitor_stop(ar);
5848 if (ret) {
5849 mutex_unlock(&ar->conf_mutex);
5850 return;
5851 }
5852
5853 arvif->is_started = false;
5854 mutex_unlock(&ar->conf_mutex);
Should this return after unlocking?
5855 }
5856
5857 ret = ath12k_mac_vdev_stop(arvif);
5858 if (ret)
5859 ath12k_warn(ab, "failed to stop vdev %i: %d\n",
5860 arvif->vdev_id, ret);
5861
5862 arvif->is_started = false;
5863
5864 if (ab->hw_params->vdev_start_delay &&
5865 arvif->vdev_type == WMI_VDEV_TYPE_MONITOR)
5866 ath12k_wmi_vdev_down(ar, arvif->vdev_id);
5867
5868 if (arvif->vdev_type != WMI_VDEV_TYPE_MONITOR &&
5869 ar->num_started_vdevs == 1 && ar->monitor_vdev_created)
5870 ath12k_mac_monitor_stop(ar);
5871
--> 5872 mutex_unlock(&ar->conf_mutex);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5873 }
regards,
dan carpenter
--
ath12k mailing list
ath12k@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/ath12k
^ permalink raw reply [flat|nested] 5+ messages in thread
* [bug report] wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices
@ 2024-06-14 17:33 Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2024-06-14 17:33 UTC (permalink / raw)
To: Kalle Valo; +Cc: ath12k
Hello Kalle Valo,
Commit d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7
devices") from Nov 28, 2022 (linux-next), leads to the following
Smatch static checker warning:
drivers/net/wireless/ath/ath12k/ce.c:303 ath12k_ce_rx_post_pipe()
error: we previously assumed 'pipe->dest_ring' could be null (see line 278)
drivers/net/wireless/ath/ath12k/ce.c
271 static int ath12k_ce_rx_post_pipe(struct ath12k_ce_pipe *pipe)
272 {
273 struct ath12k_base *ab = pipe->ab;
274 struct sk_buff *skb;
275 dma_addr_t paddr;
276 int ret = 0;
277
278 if (!(pipe->dest_ring || pipe->status_ring))
This is the same as:
if (!pipe->dest_ring && !pipe->status_ring)
Imagine that ->dest_ring is NULL but ->status_ring isn't.
279 return 0;
280
281 spin_lock_bh(&ab->ce.ce_lock);
282 while (pipe->rx_buf_needed) {
283 skb = dev_alloc_skb(pipe->buf_sz);
284 if (!skb) {
285 ret = -ENOMEM;
286 goto exit;
287 }
288
289 WARN_ON_ONCE(!IS_ALIGNED((unsigned long)skb->data, 4));
290
291 paddr = dma_map_single(ab->dev, skb->data,
292 skb->len + skb_tailroom(skb),
293 DMA_FROM_DEVICE);
294 if (unlikely(dma_mapping_error(ab->dev, paddr))) {
295 ath12k_warn(ab, "failed to dma map ce rx buf\n");
296 dev_kfree_skb_any(skb);
297 ret = -EIO;
298 goto exit;
299 }
300
301 ATH12K_SKB_RXCB(skb)->paddr = paddr;
302
--> 303 ret = ath12k_ce_rx_buf_enqueue_pipe(pipe, skb, paddr);
^^^^
Unchecked dereference of pipe->dest_ring inside the function.
304 if (ret) {
305 ath12k_warn(ab, "failed to enqueue rx buf: %d\n", ret);
306 dma_unmap_single(ab->dev, paddr,
307 skb->len + skb_tailroom(skb),
308 DMA_FROM_DEVICE);
309 dev_kfree_skb_any(skb);
310 goto exit;
311 }
312 }
313
314 exit:
315 spin_unlock_bh(&ab->ce.ce_lock);
316 return ret;
317 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-06-14 17:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-16 12:28 [bug report] wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2024-06-14 17:33 Dan Carpenter
2023-02-16 11:54 Dan Carpenter
2023-02-16 11:44 Dan Carpenter
2023-02-16 9:57 Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox