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

[Kalle Valo <kvalo@kernel.org>]

Aditya Kumar Singh <quic_adisi@quicinc.com> writes:

> On 8/6/24 17:58, Kalle Valo wrote:
>
>> 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.
>
> deflink is not a pointer member. It is statically defined inside
> ahvif. So basically this memsets the whole deflink memory region to 0.
>> 
>>> -	arvif->vif = vif;
>>> +	ahvif->ah = ah;
>>> +	ahvif->vif = vif;
>>> +	arvif = &ahvif->deflink;
>> So here we assign arvif to NULL.
>
> This would be a still valid pointer.
>> 
>>> +	arvif->ahvif = ahvif;
>> And because arvif is NULL this is a null pointer reference, right?
>> Or am
>> I missing something?
>
> So since it is valid pointer, this is not a NULL pointer de-reference.

Ah, of course. Clearly my vacation was too good and I forgot C :) Thanks!

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

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