From 276cbb69f3e97073aaca3221a1fd8d09424bc116 Mon Sep 17 00:00:00 2001 From: Nachiket Kukade Date: Sat, 9 May 2020 17:04:40 +0530 Subject: [PATCH] wpa_supplicant: Fix memory leaks in WPA3 connection 1. Buffers for SAE messages are not freed after the handshake. This causes memory leak, free buffers after SAE handshake. 2. SAE global data is not freed until the next WPA3 connection takes place, holding up heap space without reason. Free theis data after SAE handshake is complete or event fails. 3. Update wifi lib which includes memory leak fix during BIP encryption/decryption operations. --- components/esp_wifi/lib | 2 +- .../src/esp_supplicant/esp_wpa3.c | 95 ++++++++++++------- .../src/esp_supplicant/esp_wpa3_i.h | 1 + .../src/esp_supplicant/esp_wpa_main.c | 2 + 4 files changed, 65 insertions(+), 35 deletions(-) diff --git a/components/esp_wifi/lib b/components/esp_wifi/lib index 4477b34a3..095cf53d1 160000 --- a/components/esp_wifi/lib +++ b/components/esp_wifi/lib @@ -1 +1 @@ -Subproject commit 4477b34a3eb506a9117c357e27b0b431ebe1d2e6 +Subproject commit 095cf53d13485144f20ceb5dd7475fd2b837c217 diff --git a/components/wpa_supplicant/src/esp_supplicant/esp_wpa3.c b/components/wpa_supplicant/src/esp_supplicant/esp_wpa3.c index abfdb3144..8abeb795a 100644 --- a/components/wpa_supplicant/src/esp_supplicant/esp_wpa3.c +++ b/components/wpa_supplicant/src/esp_supplicant/esp_wpa3.c @@ -21,19 +21,25 @@ static struct sae_data g_sae_data; static struct wpabuf *g_sae_token = NULL; +static struct wpabuf *g_sae_commit = NULL; +static struct wpabuf *g_sae_confirm = NULL; int g_allowed_groups[] = { IANA_SECP256R1, 0 }; -static struct wpabuf *wpa3_build_sae_commit(u8 *bssid) +static esp_err_t wpa3_build_sae_commit(u8 *bssid) { int default_group = IANA_SECP256R1; - struct wpabuf *buf; u32 len = 0; u8 own_addr[ETH_ALEN]; const u8 *pw; if (wpa_sta_is_cur_pmksa_set()) { wpa_printf(MSG_INFO, "wpa3: Skip SAE and use cached PMK instead"); - return NULL; + return ESP_FAIL; + } + + if (g_sae_commit) { + wpabuf_free(g_sae_commit); + g_sae_commit = NULL; } if (g_sae_token) { @@ -44,33 +50,34 @@ static struct wpabuf *wpa3_build_sae_commit(u8 *bssid) memset(&g_sae_data, 0, sizeof(g_sae_data)); if (sae_set_group(&g_sae_data, default_group)) { wpa_printf(MSG_ERROR, "wpa3: could not set SAE group %d", default_group); - return NULL; + return ESP_FAIL; } esp_wifi_get_macaddr_internal(WIFI_IF_STA, own_addr); if (!bssid) { wpa_printf(MSG_ERROR, "wpa3: cannot prepare SAE commit with no BSSID!"); - return NULL; + return ESP_FAIL; } pw = (const u8 *)esp_wifi_sta_get_prof_password_internal(); if (sae_prepare_commit(own_addr, bssid, pw, strlen((const char *)pw), NULL, &g_sae_data) < 0) { wpa_printf(MSG_ERROR, "wpa3: failed to prepare SAE commit!"); - return NULL; + return ESP_FAIL; } reuse_data: len += SAE_COMMIT_MAX_LEN; - buf = wpabuf_alloc(len); - if (!buf) { + g_sae_commit = wpabuf_alloc(len); + if (!g_sae_commit) { wpa_printf(MSG_ERROR, "wpa3: failed to allocate buffer for commit msg"); - return NULL; + return ESP_FAIL; } - if (sae_write_commit(&g_sae_data, buf, g_sae_token, NULL) != ESP_OK) { + if (sae_write_commit(&g_sae_data, g_sae_commit, g_sae_token, NULL) != ESP_OK) { wpa_printf(MSG_ERROR, "wpa3: failed to write SAE commit msg"); - wpabuf_free(buf); - return NULL; + wpabuf_free(g_sae_commit); + g_sae_commit = NULL; + return ESP_FAIL; } if (g_sae_token) { @@ -79,53 +86,72 @@ reuse_data: } g_sae_data.state = SAE_COMMITTED; - return buf; + return ESP_OK; } -static struct wpabuf *wpa3_build_sae_confirm(void) +static esp_err_t wpa3_build_sae_confirm(void) { - struct wpabuf *buf; - if (g_sae_data.state != SAE_COMMITTED) - return NULL; + return ESP_FAIL; - buf = wpabuf_alloc(SAE_COMMIT_MAX_LEN); - if (!buf) { - wpa_printf(MSG_ERROR, "wpa3: failed to allocate buffer for confirm msg"); - return NULL; + if (g_sae_confirm) { + wpabuf_free(g_sae_confirm); + g_sae_confirm = NULL; } - if (sae_write_confirm(&g_sae_data, buf) != ESP_OK) { + g_sae_confirm = wpabuf_alloc(SAE_COMMIT_MAX_LEN); + if (!g_sae_confirm) { + wpa_printf(MSG_ERROR, "wpa3: failed to allocate buffer for confirm msg"); + return ESP_FAIL; + } + + if (sae_write_confirm(&g_sae_data, g_sae_confirm) != ESP_OK) { wpa_printf(MSG_ERROR, "wpa3: failed to write SAE confirm msg"); - wpabuf_free(buf); - return NULL; + wpabuf_free(g_sae_confirm); + g_sae_confirm = NULL; + return ESP_FAIL; } g_sae_data.state = SAE_CONFIRMED; - return buf; + return ESP_OK; +} + +void esp_wpa3_free_sae_data(void) +{ + if (g_sae_commit) { + wpabuf_free(g_sae_commit); + g_sae_commit = NULL; + } + + if (g_sae_confirm) { + wpabuf_free(g_sae_confirm); + g_sae_confirm = NULL; + } + sae_clear_data(&g_sae_data); } static u8 *wpa3_build_sae_msg(u8 *bssid, u32 sae_msg_type, u32 *sae_msg_len) { - struct wpabuf *buf = NULL; + u8 *buf = NULL; switch (sae_msg_type) { case SAE_MSG_COMMIT: - buf = wpa3_build_sae_commit(bssid); + if (ESP_OK != wpa3_build_sae_commit(bssid)) + return NULL; + *sae_msg_len = (u32)wpabuf_len(g_sae_commit); + buf = wpabuf_mhead_u8(g_sae_commit); break; case SAE_MSG_CONFIRM: - buf = wpa3_build_sae_confirm(); + if (ESP_OK != wpa3_build_sae_confirm()) + return NULL; + *sae_msg_len = (u32)wpabuf_len(g_sae_confirm); + buf = wpabuf_mhead_u8(g_sae_confirm); break; default: break; } - if (buf) { - *sae_msg_len = (u32)wpabuf_len(buf); - return wpabuf_mhead_u8(buf); - } else { - return NULL; - } + return buf; } static int wpa3_parse_sae_commit(u8 *buf, u32 len, u16 status) @@ -190,6 +216,7 @@ static int wpa3_parse_sae_msg(u8 *buf, u32 len, u32 sae_msg_type, u16 status) break; case SAE_MSG_CONFIRM: ret = wpa3_parse_sae_confirm(buf, len); + esp_wpa3_free_sae_data(); break; default: wpa_printf(MSG_ERROR, "wpa3: Invalid SAE msg type(%d)!", sae_msg_type); diff --git a/components/wpa_supplicant/src/esp_supplicant/esp_wpa3_i.h b/components/wpa_supplicant/src/esp_supplicant/esp_wpa3_i.h index ffbb19e6c..93223040e 100644 --- a/components/wpa_supplicant/src/esp_supplicant/esp_wpa3_i.h +++ b/components/wpa_supplicant/src/esp_supplicant/esp_wpa3_i.h @@ -21,6 +21,7 @@ #ifdef CONFIG_WPA3_SAE void esp_wifi_register_wpa3_cb(struct wpa_funcs *wpa_cb); +void esp_wpa3_free_sae_data(void); #else /* CONFIG_WPA3_SAE */ diff --git a/components/wpa_supplicant/src/esp_supplicant/esp_wpa_main.c b/components/wpa_supplicant/src/esp_supplicant/esp_wpa_main.c index bbb40e9aa..23ab60baa 100644 --- a/components/wpa_supplicant/src/esp_supplicant/esp_wpa_main.c +++ b/components/wpa_supplicant/src/esp_supplicant/esp_wpa_main.c @@ -26,6 +26,7 @@ #include "esp_wpas_glue.h" #include "esp_hostap.h" +#include "esp_system.h" #include "crypto/crypto.h" #include "crypto/sha1.h" #include "crypto/aes_wrap.h" @@ -188,6 +189,7 @@ static void wpa_sta_disconnected_cb(uint8_t reason_code) case WIFI_REASON_AUTH_FAIL: case WIFI_REASON_ASSOC_FAIL: case WIFI_REASON_CONNECTION_FAIL: + esp_wpa3_free_sae_data(); wpa_sta_clear_curr_pmksa(); break; default: