Hi Dan,
> Hello Emmanuel Grumbach,
>
> The patch 6d19a5eba5cd: "iwlwifi: integrate with iwlmei" from Nov 12, 2021,
> leads to the following Smatch static checker warning:
>
> drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c:2640
> iwl_mvm_start_ap_ibss()
> error: NULL dereference inside function
> '__iwl_mvm_mac_set_key()'
Where in __iwl_mvm_mac_set_key() ?
This function should be able to cope with with a NULL sta I think.
I don't really see how this could be related to my patch since iwlmei is not related to AP mode at all.
I also moved to that commit, but the line numbers don't match, so I am a bit confused.
>
> drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
> 2559 static int iwl_mvm_start_ap_ibss(struct ieee80211_hw *hw,
> 2560 struct ieee80211_vif *vif)
> 2561 {
> 2562 struct iwl_mvm *mvm = IWL_MAC80211_GET_MVM(hw);
> 2563 struct iwl_mvm_vif *mvmvif = iwl_mvm_vif_from_mac80211(vif);
> 2564 int ret, i;
> 2565
> 2566 mutex_lock(&mvm->mutex);
> 2567
> 2568 /* Send the beacon template */
> 2569 ret = iwl_mvm_mac_ctxt_beacon_changed(mvm, vif);
> 2570 if (ret)
> 2571 goto out_unlock;
> 2572
> 2573 /*
> 2574 * Re-calculate the tsf id, as the leader-follower relations depend
> on
> 2575 * the beacon interval, which was not known when the AP
> interface
> 2576 * was added.
> 2577 */
> 2578 if (vif->type == NL80211_IFTYPE_AP)
> 2579 iwl_mvm_mac_ctxt_recalc_tsf_id(mvm, vif);
> 2580
> 2581 mvmvif->ap_assoc_sta_count = 0;
> 2582
> 2583 /* Add the mac context */
> 2584 ret = iwl_mvm_mac_ctxt_add(mvm, vif);
> 2585 if (ret)
> 2586 goto out_unlock;
> 2587
> 2588 /* Perform the binding */
> 2589 ret = iwl_mvm_binding_add_vif(mvm, vif);
> 2590 if (ret)
> 2591 goto out_remove;
> 2592
> 2593 /*
> 2594 * This is not very nice, but the simplest:
> 2595 * For older FWs adding the mcast sta before the bcast station
> may
> 2596 * cause assert 0x2b00.
> 2597 * This is fixed in later FW so make the order of removal depend
> on
> 2598 * the TLV
> 2599 */
> 2600 if (fw_has_api(&mvm->fw->ucode_capa,
> IWL_UCODE_TLV_API_STA_TYPE)) {
> 2601 ret = iwl_mvm_add_mcast_sta(mvm, vif);
> 2602 if (ret)
> 2603 goto out_unbind;
> 2604 /*
> 2605 * Send the bcast station. At this stage the TBTT and DTIM
> time
> 2606 * events are added and applied to the scheduler
> 2607 */
> 2608 ret = iwl_mvm_send_add_bcast_sta(mvm, vif);
> 2609 if (ret) {
> 2610 iwl_mvm_rm_mcast_sta(mvm, vif);
> 2611 goto out_unbind;
> 2612 }
> 2613 } else {
> 2614 /*
> 2615 * Send the bcast station. At this stage the TBTT and DTIM
> time
> 2616 * events are added and applied to the scheduler
> 2617 */
> 2618 ret = iwl_mvm_send_add_bcast_sta(mvm, vif);
> 2619 if (ret)
> 2620 goto out_unbind;
> 2621 ret = iwl_mvm_add_mcast_sta(mvm, vif);
> 2622 if (ret) {
> 2623 iwl_mvm_send_rm_bcast_sta(mvm, vif);
> 2624 goto out_unbind;
> 2625 }
> 2626 }
> 2627
> 2628 /* must be set before quota calculations */
> 2629 mvmvif->ap_ibss_active = true;
> 2630
> 2631 /* send all the early keys to the device now */
> 2632 for (i = 0; i < ARRAY_SIZE(mvmvif->ap_early_keys); i++) {
> 2633 struct ieee80211_key_conf *key = mvmvif->ap_early_keys[i];
> 2634
> 2635 if (!key)
> 2636 continue;
> 2637
> 2638 mvmvif->ap_early_keys[i] = NULL;
> 2639
> --> 2640 ret = __iwl_mvm_mac_set_key(hw, SET_KEY, vif, NULL,
> key);
> ^^^^ This passes a NULL "sta" and now it
> will always crash. (Possibly it used to sometimes crash before your patch but
> the static checker does not mind about that. :P).
This existed long before my patch
>
> 2641 if (ret)
> 2642 goto out_quota_failed;
> 2643 }
> 2644
> 2645 if (vif->type == NL80211_IFTYPE_AP && !vif->p2p) {
> 2646 iwl_mvm_vif_set_low_latency(mvmvif, true,
> 2647 LOW_LATENCY_VIF_TYPE);
> 2648 iwl_mvm_send_low_latency_cmd(mvm, true, mvmvif->id);
> 2649 }
> 2650
> 2651 /* power updated needs to be done before quotas */
> 2652 iwl_mvm_power_update_mac(mvm);
> 2653
> 2654 ret = iwl_mvm_update_quotas(mvm, false, NULL);
> 2655 if (ret)
> 2656 goto out_quota_failed;
> 2657
> 2658 /* Need to update the P2P Device MAC (only GO, IBSS is single
> vif) */
> 2659 if (vif->p2p && mvm->p2p_device_vif)
> 2660 iwl_mvm_mac_ctxt_changed(mvm, mvm->p2p_device_vif,
> false, NULL);
> 2661
> 2662 iwl_mvm_bt_coex_vif_change(mvm);
> 2663
> 2664 /* we don't support TDLS during DCM */
> 2665 if (iwl_mvm_phy_ctx_count(mvm) > 1)
> 2666 iwl_mvm_teardown_tdls_peers(mvm);
> 2667
> 2668 iwl_mvm_ftm_restart_responder(mvm, vif);
> 2669
> 2670 goto out_unlock;
> 2671
> 2672 out_quota_failed:
> 2673 iwl_mvm_power_update_mac(mvm);
> 2674 mvmvif->ap_ibss_active = false;
> 2675 iwl_mvm_send_rm_bcast_sta(mvm, vif);
> 2676 iwl_mvm_rm_mcast_sta(mvm, vif);
> 2677 out_unbind:
> 2678 iwl_mvm_binding_remove_vif(mvm, vif);
> 2679 out_remove:
> 2680 iwl_mvm_mac_ctxt_remove(mvm, vif);
> 2681 out_unlock:
> 2682 mutex_unlock(&mvm->mutex);
> 2683 return ret;
> 2684 }
>
> regards,
> dan carpenter
