Re: [PATCH v6 1/3] wifi: ath12k: prepare vif data structure for MLO handling

[Kalle Valo <kvalo@kernel.org>]

Rameshkumar Sundaram <quic_ramess@quicinc.com> writes:

> From: Sriram R <quic_srirrama@quicinc.com>
>
> To prepare the driver for MLO support, split the driver vif
> data structure to scale for multiple links. This requires changing
> the use of arvif to per link and not per hw which can now
> comprise of multiple links.
> Also since most configurations from mac80211 are done per link, do refactoring
> of the driver functions to apply these configurations at link level.

Something I noticed while reviewing this patchset:

>  static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>  				       struct ieee80211_vif *vif)
>  {
> -	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
> +	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
> +	struct ath12k_vif *ahvif = ath12k_vif_to_ahvif(vif);
> +	struct ath12k_link_vif *arvif;
>  	int i;
>  
> -	memset(arvif, 0, sizeof(*arvif));
> +	mutex_lock(&ah->conf_mutex);
> +	memset(ahvif, 0, sizeof(*ahvif));

Here we are we clearing ahvif so ahvif->deflink is NULL.

> -	arvif->vif = vif;
> +	ahvif->ah = ah;
> +	ahvif->vif = vif;
> +	arvif = &ahvif->deflink;

So here we assign arvif to NULL.

> +	arvif->ahvif = ahvif;

And because arvif is NULL this is a null pointer reference, right? Or am
I missing something?

I'm still reviewing the patchset, please don't submit a new version
until I have said that the review is done.

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

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