Hi Michal,
thanks for the comments ..
On Thu, Mar 17, 2016 at 12:35:00PM +0100, Michal Kazior wrote:
> On 17 March 2016 at 12:20, Mohammed Shafi Shajakhan
> <mohammed@xxxxxxxxxxxxxx> wrote:
> > Hi Michal,
> >
> > On Thu, Mar 17, 2016 at 12:12:31PM +0100, Michal Kazior wrote:
> >> On 17 March 2016 at 11:48, Mohammed Shafi Shajakhan
> >> <mohammed@xxxxxxxxxxxxxxxx> wrote:
> >> [...]
> >> > +void ath10k_sta_statistics(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> >> > + struct ieee80211_sta *sta,
> >> > + struct station_info *sinfo)
> >> > +{
> >> > + struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
> >> > + struct ath10k *ar = arsta->arvif->ar;
> >> > +
> >> > + mutex_lock(&ar->conf_mutex);
> >> > +
> >> > + if (ar->state != ATH10K_STATE_ON &&
> >> > + ar->state != ATH10K_STATE_RESTARTED)
> >> > + goto out;
> >>
> >> Do you really need mutex and ar->state check in this function?
> >>
> >
> > [shafi] By default peer stats will be disabled, we are enabling this by debugfs
> > (hw-restart) so i thought these checks are needed , please advise .. Do you say
> > they will be never hit
>
> Hmm.. I think mac80211 shouldn't call sta_statistics() before
> sta_state() during recovery (it makes no sense). In practice I think
> this isn't enforced in which case it's a mac80211 bug.
[shafi] sure i will check this. If the hardware is restarting there should
be no stations connected and station related info.
>
> Anyway, this isn't much of a problem now. You only read out u64 from
> `arsta` (sta->drv_priv). Even if it's uninitialized/undefined there's
> no way for you to crash the system (it's not a list, spinlock or any
> other complex data structure). Worst case userspace will get garbage
> rx_duration value if it happens to get_station() while hw-restart is
> ongoing.
[shafi] will check this ..
>
> It'd be good to verify this is actually a problem and - assuming you
> want to guarantee correct readouts at all times - to fix the problem
> in mac80211.
>
[shafi] ok, sure
-shafi
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
