On Wed, Feb 24, 2016 at 12:53:20PM +0100, michael-dev@xxxxxxxxxxxxx wrote:
> diff --git a/src/ap/ieee802_11.c b/src/ap/ieee802_11.c
> +int handle_auth_cfg_sta(struct hostapd_data *hapd, struct sta_info *sta,
> + int res, struct hostapd_allowed_address_info *info,
> + u16 *resp)
> + hostapd_free_psk_list(sta->psk);
> + if (hapd->conf->wpa_psk_radius != PSK_RADIUS_IGNORED) {
> + sta->psk = info->psk;
> + info->psk = NULL;
> + } else {
> + sta->psk = NULL;
> + }
Wouldn't this be able to leave out info->psk allocated and then return
0?
> + sta->identity = info->identity;
> + info->identity = NULL;
> + sta->radius_cui = info->radius_cui;
> + info->radius_cui = NULL;
These free the other allocated items within struct
hostapd_allowed_address_info, but this is done without calling
hostapd_allowed_address_free() which can make it more likely for this
detail to be missed if new fields are added in the future.
> + return 0;
So this is handle_auth_cfg_sta() returning 0 with info->psk potentially
left allocated.
> static void handle_auth(struct hostapd_data *hapd,
> const struct ieee80211_mgmt *mgmt, size_t len)
> {
> @@ -1068,9 +1129,8 @@ static void handle_auth(struct hostapd_data *hapd,
> res = hostapd_allowed_address(hapd, mgmt->sa, (u8 *) mgmt, len,
> - &session_timeout,
> - &acct_interim_interval, &vlan_id,
> - &psk, &identity, &radius_cui);
> + &handle_auth_restart_cb,
> + &details);
This allocates the details.* based on response.
> @@ -1085,6 +1145,7 @@ static void handle_auth(struct hostapd_data *hapd,
> + hostapd_allowed_address_free(&details);
> return;
And many code paths within handle_auth() were modified to call
hostapd_allowed_address_free() before returning.
> @@ -1138,47 +1202,12 @@ static void handle_auth(struct hostapd_data *hapd,
> + if (handle_auth_cfg_sta(hapd, sta, res, &details, &resp) < 0)
> goto fail;
But after this, only the "goto fail" cases end up calling
hostapd_allowed_address_free().
Is this correct?
> @@ -1288,9 +1317,7 @@ static void handle_auth(struct hostapd_data *hapd,
> fail:
> - os_free(identity);
> - os_free(radius_cui);
> - hostapd_free_psk_list(psk);
> + hostapd_allowed_address_free(&details);
So there is the hostapd_allowed_address_free() for some of the error
cases and the success path. However, there are at least two return
statements above in WLAN_AUTH_FT and WLAN_AUTH_SAE cases of
switch (auth_alg). Do those free memory in all cases? Even if they do,
it looks a bit strange to see paths that do not have
hostapd_allowed_address_free() being called.
--
Jouni Malinen PGP id EFC895FA
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
