From c5fee1604bdd52e4325b636a180309c3741e3a6d Mon Sep 17 00:00:00 2001 From: Michael Braun Date: Thu, 23 Mar 2017 12:57:18 +0100 Subject: [PATCH] FT: Schedule wpa_ft_rrb_rx() through eloop in intra-process communication With AP-AP communication, when hapd0 sends a packet, hapd1 can receive it immediately and send a response. But hapd0 will only read and process the response after it has returned from the sending context, that is entered eloop again. So one does not need to consider the RX function of the reply to run for the request sending hapd before the send calling function has returned. Previously, with intra-process communication, the packet is not scheduled through eloop. Thus the RX handler of the reply might be run while the sending context of the original request has not returned. This might become problematic, e.g., when deferring a management frame processing until an RRB response is received and then have the request restarted and finished before the original request handling has been stopped. I'm not aware of any concrete bug this is currently triggering but came across it while thinking of FT RRB AP-AP sequence numbering. I think the non-eloop scheduling approach might be error-prone and thus propose to model it more closely to the way the message would be received from a socket. Additionally, this ensures that the tests model AP-AP communication more closely to real world. Solution: queue these packets through eloop. Signed-off-by: Michael Braun --- src/ap/hostapd.c | 3 ++ src/ap/hostapd.h | 5 +++ src/ap/wpa_auth_glue.c | 78 ++++++++++++++++++++++++++++++++++-------- 3 files changed, 72 insertions(+), 14 deletions(-) diff --git a/src/ap/hostapd.c b/src/ap/hostapd.c index 3c5a56c52..01215aace 100644 --- a/src/ap/hostapd.c +++ b/src/ap/hostapd.c @@ -2017,6 +2017,9 @@ hostapd_alloc_bss_data(struct hostapd_iface *hapd_iface, dl_list_init(&hapd->ctrl_dst); dl_list_init(&hapd->nr_db); hapd->dhcp_sock = -1; +#ifdef CONFIG_IEEE80211R_AP + dl_list_init(&hapd->l2_queue); +#endif /* CONFIG_IEEE80211R_AP */ return hapd; } diff --git a/src/ap/hostapd.h b/src/ap/hostapd.h index 5ab623de1..9d9eb6df3 100644 --- a/src/ap/hostapd.h +++ b/src/ap/hostapd.h @@ -185,6 +185,11 @@ struct hostapd_data { #endif /* CONFIG_FULL_DYNAMIC_VLAN */ struct l2_packet_data *l2; + +#ifdef CONFIG_IEEE80211R_AP + struct dl_list l2_queue; +#endif /* CONFIG_IEEE80211R_AP */ + struct wps_context *wps; int beacon_set_done; diff --git a/src/ap/wpa_auth_glue.c b/src/ap/wpa_auth_glue.c index d4e014079..9bd07c8e5 100644 --- a/src/ap/wpa_auth_glue.c +++ b/src/ap/wpa_auth_glue.c @@ -9,6 +9,8 @@ #include "utils/includes.h" #include "utils/common.h" +#include "utils/eloop.h" +#include "utils/list.h" #include "common/ieee802_11_defs.h" #include "common/sae.h" #include "common/wpa_ctrl.h" @@ -417,6 +419,31 @@ static int hostapd_wpa_auth_for_each_auth( #ifdef CONFIG_IEEE80211R_AP +struct wpa_ft_rrb_rx_later_data { + struct dl_list list; + u8 addr[ETH_ALEN]; + size_t data_len; + /* followed by data_len octets of data */ +}; + +static void hostapd_wpa_ft_rrb_rx_later(void *eloop_ctx, void *timeout_ctx) +{ + struct hostapd_data *hapd = eloop_ctx; + struct wpa_ft_rrb_rx_later_data *data, *n; + + dl_list_for_each_safe(data, n, &hapd->l2_queue, + struct wpa_ft_rrb_rx_later_data, list) { + if (hapd->wpa_auth) { + wpa_ft_rrb_rx(hapd->wpa_auth, data->addr, + (const u8 *) (data + 1), + data->data_len); + } + dl_list_del(&data->list); + os_free(data); + } +} + + struct wpa_auth_ft_iface_iter_data { struct hostapd_data *src_hapd; const u8 *dst; @@ -428,27 +455,48 @@ struct wpa_auth_ft_iface_iter_data { static int hostapd_wpa_auth_ft_iter(struct hostapd_iface *iface, void *ctx) { struct wpa_auth_ft_iface_iter_data *idata = ctx; + struct wpa_ft_rrb_rx_later_data *data; struct hostapd_data *hapd; size_t j; for (j = 0; j < iface->num_bss; j++) { hapd = iface->bss[j]; - if (hapd == idata->src_hapd) + if (hapd == idata->src_hapd || + !hapd->wpa_auth || + os_memcmp(hapd->own_addr, idata->dst, ETH_ALEN) != 0) continue; - if (!hapd->wpa_auth) - continue; - if (os_memcmp(hapd->own_addr, idata->dst, ETH_ALEN) == 0) { - wpa_printf(MSG_DEBUG, "FT: Send RRB data directly to " - "locally managed BSS " MACSTR "@%s -> " - MACSTR "@%s", - MAC2STR(idata->src_hapd->own_addr), - idata->src_hapd->conf->iface, - MAC2STR(hapd->own_addr), hapd->conf->iface); - wpa_ft_rrb_rx(hapd->wpa_auth, - idata->src_hapd->own_addr, - idata->data, idata->data_len); + + wpa_printf(MSG_DEBUG, + "FT: Send RRB data directly to locally managed BSS " + MACSTR "@%s -> " MACSTR "@%s", + MAC2STR(idata->src_hapd->own_addr), + idata->src_hapd->conf->iface, + MAC2STR(hapd->own_addr), hapd->conf->iface); + + /* Defer wpa_ft_rrb_rx() until next eloop step as this is + * when it would be triggered when reading from a socket. + * This avoids + * hapd0:send -> hapd1:recv -> hapd1:send -> hapd0:recv, + * that is calling hapd0:recv handler from within + * hapd0:send directly. + */ + data = os_zalloc(sizeof(*data) + idata->data_len); + if (!data) return 1; - } + + os_memcpy(data->addr, idata->src_hapd->own_addr, ETH_ALEN); + os_memcpy(data + 1, idata->data, idata->data_len); + data->data_len = idata->data_len; + + dl_list_add(&hapd->l2_queue, &data->list); + + if (!eloop_is_timeout_registered(hostapd_wpa_ft_rrb_rx_later, + hapd, NULL)) + eloop_register_timeout(0, 0, + hostapd_wpa_ft_rrb_rx_later, + hapd, NULL); + + return 1; } return 0; @@ -716,6 +764,8 @@ void hostapd_deinit_wpa(struct hostapd_data *hapd) ieee802_1x_deinit(hapd); #ifdef CONFIG_IEEE80211R_AP + eloop_cancel_timeout(hostapd_wpa_ft_rrb_rx_later, hapd, ELOOP_ALL_CTX); + hostapd_wpa_ft_rrb_rx_later(hapd, NULL); /* flush without delivering */ l2_packet_deinit(hapd->l2); hapd->l2 = NULL; #endif /* CONFIG_IEEE80211R_AP */