Patchwork [v2] nl80211: fix EAPOL frames not being delivered

login
register
mail settings
Submitter Maxime Bizon
Date March 20, 2014, 6:29 p.m.
Message ID <1395340184-5035-1-git-send-email-mbizon@freebox.fr>
Download mbox | patch
Permalink /patch/332357/
State Accepted
Headers show

Comments

Maxime Bizon - March 20, 2014, 6:29 p.m.
When hostapd choose to reuse an existing interface, it does not add it
to the set of interfaces from which we accept EAPOL packets.

Make sure we always add it to that set.

Signed-off-by: Maxime Bizon <mbizon@freebox.fr>
---
 src/drivers/driver_nl80211.c |    3 +++
 1 file changed, 3 insertions(+)
Jouni Malinen - March 25, 2014, 10:52 p.m.
On Thu, Mar 20, 2014 at 07:29:44PM +0100, Maxime Bizon wrote:
> When hostapd choose to reuse an existing interface, it does not add it
> to the set of interfaces from which we accept EAPOL packets.
> 
> Make sure we always add it to that set.

> diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
> @@ -9862,6 +9862,9 @@ static int wpa_driver_nl80211_if_add(void *priv, enum wpa_driver_if_type type,
>  	if (drv->global)
>  		drv->global->if_add_ifindex = ifidx;
>  
> +	if (drv->nlmode != NL80211_IFTYPE_P2P_DEVICE)
> +		add_ifidx(drv, ifidx);
> +
>  	return 0;

Adding a call to add_ifidx() makes sense, but not in this location. This
would end up adding interfaces to the list multiple times and then being
left there even after del_ifidx() call which is removing only a single
entry. I'd assume this new add_ifidx() call should be much earlier in
this function. To be more exact, where the ifdex == -ENFILE is checked.
Maxime Bizon - March 26, 2014, 7:19 a.m.
On Wednesday 26 Mar 2014 à 00:52:17 (+0200), Jouni Malinen wrote:

> Adding a call to add_ifidx() makes sense, but not in this location. This
> would end up adding interfaces to the list multiple times and then being

you're right, I misread the first for loop in add_ifidx() and
thought it was an opencoded has_ifidx() to avoid duplicate entries

what about adding has_ifidx() at the beginning ?

> left there even after del_ifidx() call which is removing only a single
> entry. I'd assume this new add_ifidx() call should be much earlier in
> this function. To be more exact, where the ifdex == -ENFILE is checked.

wpa_driver_nl80211_if_add() has many error paths, and I did not want
to add 'if (added) del_ifidx()' everywhere
Jouni Malinen - March 27, 2014, 2:43 p.m.
On Thu, Mar 20, 2014 at 07:29:44PM +0100, Maxime Bizon wrote:
> When hostapd choose to reuse an existing interface, it does not add it
> to the set of interfaces from which we accept EAPOL packets.
> 
> Make sure we always add it to that set.

Thanks, applied with the changes discussed on the list.

Patch

diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index fb2847f..7fb7d3d 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -9862,6 +9862,9 @@  static int wpa_driver_nl80211_if_add(void *priv, enum wpa_driver_if_type type,
 	if (drv->global)
 		drv->global->if_add_ifindex = ifidx;
 
+	if (drv->nlmode != NL80211_IFTYPE_P2P_DEVICE)
+		add_ifidx(drv, ifidx);
+
 	return 0;
 }