On Tue, Apr 12, 2016 at 08:34:08AM +0200, Martin Pieuchot wrote:
> As reported by jsg@ the wifi stack should use if_start() just like the
> rest of the kernel.  One of the patterns can even be converted to
> if_enqueue().

Note that if_enqueue() checks for a bridge port and, if present, sends
frames to the bridge unless the M_PROTO1 flag is set on the mbuf.
This looks like a potential change in behaviour.

In case the wifi interface is in a bridge, could we end up sending the
EAPOL key packet to members of that bridge? Would this be a problem?

These are unicast frames. I'm not entirely sure what the bridge would
really do here. It seems unlikely that a situation where it hasn't already
learned the WPA peer's MAC is even possible, i.e. where it would broadcast
the frame to all members. But if this did happen, could it expose bugs where
(ethernet) drivers for other bridge members choke on IEEE 802.11 frames?

It looks like we could set the M_PROTO1 flag to skip the bridge just
to be sure it won't interfere.
But I don't know what other side effects this may have.

> diff -u -p -r1.26 ieee80211_pae_output.c
> --- net80211/ieee80211_pae_output.c   25 Nov 2015 03:10:00 -0000      1.26
> +++ net80211/ieee80211_pae_output.c   12 Apr 2016 06:25:59 -0000
> @@ -66,7 +66,7 @@ ieee80211_send_eapol_key(struct ieee8021
>       struct ether_header *eh;
>       struct ieee80211_eapol_key *key;
>       u_int16_t info;
> -     int s, len, error;
> +     int len;
>  
>       M_PREPEND(m, sizeof(struct ether_header), M_DONTWAIT);
>       if (m == NULL)
> @@ -118,22 +118,12 @@ ieee80211_send_eapol_key(struct ieee8021
>       if (info & EAPOL_KEY_KEYMIC)
>               ieee80211_eapol_key_mic(key, ptk->kck);
>  
> -     len = m->m_pkthdr.len;
> -     s = splnet();
>  #ifndef IEEE80211_STA_ONLY
>       /* start a 100ms timeout if an answer is expected from supplicant */
>       if (info & EAPOL_KEY_KEYACK)
>               timeout_add_msec(&ni->ni_eapol_to, 100);
>  #endif
> -     IFQ_ENQUEUE(&ifp->if_snd, m, error);
> -     if (error == 0) {
> -             ifp->if_obytes += len;
> -             if (!ifq_is_oactive(&ifp->if_snd))
> -                     (*ifp->if_start)(ifp);
> -     }
> -     splx(s);
> -
> -     return error;
> +     return if_enqueue(ifp, m);
>  }
>  
>  #ifndef IEEE80211_STA_ONLY

Reply via email to