On Thu, 2023-01-26 at 17:53 +0100, Jonas Jelonek wrote: > > > > On Tue, 2022-09-20 at 12:40 +0200, Jonas Jelonek wrote: > > > @@ -4846,16 +4989,32 @@ static int > > > hwsim_tx_info_frame_received_nl(struct sk_buff *skb_2, > > > > > > tx_attempts = (struct hwsim_tx_rate *)nla_data( > > > info->attrs[HWSIM_ATTR_TX_INFO]); > > > + tx_attempts_flags = (struct hwsim_tx_rate_flag *)nla_data( > > > + info->attrs[HWSIM_ATTR_TX_INFO_FLAGS]); > > > + sta = (struct ieee80211_sta *)txi->rate_driver_data[1]; > > > > That seems dangerous - what if the STA was freed already? You don't > > walk > > the pending list or something if the STA goes away. > > Yes, I see. Is it in general a bad idea to take the sta reference from > ieee80211_control, put > it in rate_driver_data and use it for tx-status? I guess I should pass > sta to tx_status_ext whenever > possible because it is used for several statistics. Well you have to think about the lifetime. In most cases you do a lookup of the STA (under RCU) etc. but > I could think of two ways: > - add NULL checks for the case that the sta pointer might be freed as > you said How would that pointer even go NULL though? The pointer would remain, but the STA can be freed, no? > - get sta by using, e.g., sta_info_get_by_addrs to get the sta if it > is available. However, this always > loops through the sta list. Might be a performance issue? It should use the hashtable? > Or do you suggest something different? Well you could keep it here and walk the list of queued skbs (?) when a STA is removed, and kill them all at that point, or something. Not sure it's worth it vs. the hash table lookup, this is just hwsim after all. johannes
