On 15/11/15(Sun) 14:34, Stefan Sperling wrote:
> On Thu, Nov 12, 2015 at 08:22:07PM +0100, Stefan Sperling wrote:
> > This diff adds an initial implementation of 802.11n.
> 
> Some of this has now been committed.
> 
> The diff below shows the outstanding parts relative to -current.
> 
>  - receiving A-MPDU and A-MSDU frames

I tried to figure out if the use-after-free could be related to the
length check in the aggregated frames but didn't find anything.

One comment below.

>  - 11n support for the iwm(4) driver (this won't go in before
>    the above part is proven stable)

I'm really concerned about the use of tasks in iwm_ampdu_rx_{start,stop}

By looking at iwn(4) which also has a lot of bits for handling A-MPDU
and A-MSDU frames, it seems to me that this go against damien@'s design.
This design might have its own set of problems, but I fear that working
around them might add another set of problems on top.

Are you sure that when these functions are called it is safe to modify
the soft states of the Block Ack agreements before notifying the
hardware?

> Index: net80211/ieee80211_node.c
> ===================================================================
> RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v
> retrieving revision 1.91
> diff -u -p -r1.91 ieee80211_node.c
> --- net80211/ieee80211_node.c 15 Nov 2015 11:14:17 -0000      1.91
> +++ net80211/ieee80211_node.c 15 Nov 2015 13:06:28 -0000
> @@ -1066,6 +1072,32 @@ ieee80211_find_node_for_beacon(struct ie
>       return (keep);
>  }
>  
> +#ifndef IEEE80211_NO_HT
> +void
> +ieee80211_ba_del(struct ieee80211_node *ni)
> +{
> +     int tid;
> +
> +     for (tid = 0; tid < nitems(ni->ni_rx_ba); tid++) {
> +             struct ieee80211_rx_ba *ba = &ni->ni_rx_ba[tid];
> +             if (ba->ba_state == IEEE80211_BA_AGREED) {
> +                     if (timeout_pending(&ba->ba_to))
> +                             timeout_del(&ba->ba_to);
> +                     ba->ba_state = IEEE80211_BA_INIT;
> +             }

I wouldn't bother with timeout_pending(), well I would event reuse the
idioms from ieee80211_recv_delba()/ieee80211_delba_request()

What about the reordering buffer?  Is it freed? (How) is this related to
ieee80211_node_leave_ht()?

Reply via email to