Skip to content

RDKBWIFI-498: Fix EM subdoc memory leaks in webconfig decode and EM a… #1250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dkyncu wants to merge 1 commit into
base: develop
Choose a base branch
from
+78 −1
Open

RDKBWIFI-498: Fix EM subdoc memory leaks in webconfig decode and EM a… #1250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion source/apps/em/wifi_em.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ static int em_sta_stats_publish(wifi_app_t *app, client_assoc_data_t *stats, int
if (rc != bus_error_success) {
wifi_util_error_print(WIFI_EM, "%s:%d: bus: bus_event_publish_fn Event failed %d\n",
__func__, __LINE__, rc);
free(data->u.encoded.raw);
free(data->u.decoded.em_sta_link_metrics_rsp.per_sta_metrics);
free(data);
return RETURN_ERR;
Expand All @@ -477,8 +478,11 @@ static int em_sta_stats_publish(wifi_app_t *app, client_assoc_data_t *stats, int
}
}

free(data->u.encoded.raw);
free(data->u.decoded.em_sta_link_metrics_rsp.per_sta_metrics);
free(data);

return RETURN_OK;
}

static int handle_ready_client_stats(wifi_app_t *app, client_assoc_data_t *stats, size_t stats_num,
Expand Down Expand Up @@ -896,10 +900,12 @@ static int em_publish_stats_data(channel_scan_response_t *scan_response)
if (status != bus_error_success) {
wifi_util_error_print(WIFI_EM, "%s:%d: bus: bus_event_publish_fn Event failed %d\n",
__func__, __LINE__, status);
free(data->u.encoded.raw);
free(data->u.decoded.collect_stats.stats);
free(data);
return RETURN_ERR;
}
free(data->u.encoded.raw);
free(data->u.decoded.collect_stats.stats);
free(data);

Expand Down Expand Up @@ -2027,7 +2033,8 @@ static int ap_report_push_cb(em_ap_report_callback_arg_t *args)
// Cleanup allocated memory
if (data != NULL) {
for (int j = 0; j < req_radio_count; j++) {
for (int i = 0; i < radio->vaps.num_vaps && i < MAX_NUM_VAP_PER_RADIO; i++) {
// num_vaps differs per radio; unused entries are NULL
for (int i = 0; i < MAX_NUM_VAP_PER_RADIO; i++) {
vap_report = &data->u.decoded.em_ap_metrics_report.radio_reports[j].vap_reports[i];
if (vap_report->sta_link_metrics != NULL) {
free(vap_report->sta_link_metrics);
Expand All @@ -2037,6 +2044,8 @@ static int ap_report_push_cb(em_ap_report_callback_arg_t *args)
}
}
}
// u.encoded.raw is NULL if encode was not reached (data is memset)
free(data->u.encoded.raw);
free(data);
Comment thread
dkyncu marked this conversation as resolved.
}

Expand Down Expand Up @@ -2948,13 +2957,16 @@ static int em_beacon_report_publish(bus_handle_t *handle, void *msg_data)
if (rc != bus_error_success) {
wifi_util_error_print(WIFI_EM, "%s:%d: bus_event_publish_fn Event failed %d\n", __func__,
__LINE__, rc);
free(wb_data->u.encoded.raw);
free(wb_data);
return RETURN_ERR;
} else {
wifi_util_dbg_print(WIFI_EM, "%s:%d: bus_event_publish_fn Event for %s\n", __func__,
__LINE__, WIFI_EM_BEACON_REPORT);
}

free(wb_data->u.encoded.raw);
free(wb_data);
return RETURN_OK;
}

Expand Down
33 changes: 33 additions & 0 deletions source/webconfig/wifi_easymesh_translator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,36 @@ unsigned int translate_auth_type_from_easymesh(unsigned int authtype)
}
}

#ifdef EM_APP
// the ap metrics report decoder mallocs per-vap sta arrays into the decoded
// params (see decode_em_ap_metrics_report_object); webconfig_data_free only
// frees u.encoded.raw, so they must be freed here after translation
static void webconfig_easymesh_free_ap_metrics_report(webconfig_subdoc_data_t *data)
{
webconfig_subdoc_decoded_data_t *decoded = &data->u.decoded;
int i, j;

if (data->type != webconfig_subdoc_type_em_ap_metrics_report) {
return;
}

for (i = 0; i < MAX_NUM_RADIOS; i++) {
for (j = 0; j < MAX_NUM_VAP_PER_RADIO; j++) {
em_vap_metrics_t *vap_report = &decoded->em_ap_metrics_report.radio_reports[i].vap_reports[j];

if (vap_report->sta_traffic_stats != NULL) {
free(vap_report->sta_traffic_stats);
vap_report->sta_traffic_stats = NULL;
}
if (vap_report->sta_link_metrics != NULL) {
free(vap_report->sta_link_metrics);
vap_report->sta_link_metrics = NULL;
}
}
}
}
#endif

// webconfig_easymesh_decode() will convert the onewifi structures to easymesh structures
webconfig_error_t webconfig_easymesh_decode(webconfig_t *config, const char *str,
webconfig_external_easymesh_t *data,
Expand All @@ -111,6 +141,9 @@ webconfig_error_t webconfig_easymesh_decode(webconfig_t *config, const char *str
wifi_util_info_print(WIFI_WEBCONFIG,"%s:%d: Easymesh decode subdoc type %d sucessfully\n", __func__, __LINE__, webconfig_easymesh_data.type);
*type = webconfig_easymesh_data.type;
//debug_external_protos(&webconfig_easymesh_data, __func__, __LINE__);
#ifdef EM_APP
webconfig_easymesh_free_ap_metrics_report(&webconfig_easymesh_data);

@Nikita-Hakai Nikita-Hakai Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think instead of this, we can cleanup in the corresponding translation function itself, something as below:

diff --git a/source/webconfig/wifi_easymesh_translator.c b/source/webconfig/wifi_easymesh_translator.c
index 5bc7ea4..fd717d1 100644
--- a/source/webconfig/wifi_easymesh_translator.c
+++ b/source/webconfig/wifi_easymesh_translator.c
@@ -1536,6 +1536,8 @@ webconfig_error_t translate_sta_link_metrics_object_to_easy_mesh_sta_info(webcon
 
     if (sta_size == 0) {
         wifi_util_error_print(WIFI_WEBCONFIG, "%s:%d Dev Stats is NULL\n", __func__, __LINE__);
+        free(params->em_sta_link_metrics_rsp.per_sta_metrics);
+        params->em_sta_link_metrics_rsp.per_sta_metrics = NULL;
         return webconfig_error_translate_to_easymesh;
     }
 
@@ -1546,6 +1548,8 @@ webconfig_error_t translate_sta_link_metrics_object_to_easy_mesh_sta_info(webcon
     proto = (webconfig_external_easymesh_t *)params->external_protos;
     if (proto == NULL) {
         wifi_util_error_print(WIFI_WEBCONFIG,"%s:%d: em_sta_info_t is NULL\n", __func__, __LINE__);
+        free(params->em_sta_link_metrics_rsp.per_sta_metrics);
+        params->em_sta_link_metrics_rsp.per_sta_metrics = NULL;
         return webconfig_error_translate_to_easymesh;
     }
 
@@ -1581,6 +1585,13 @@ webconfig_error_t translate_sta_link_metrics_object_to_easy_mesh_sta_info(webcon
             proto->put_sta_info(proto->data_model, em_sta_dev_info, em_target_sta_map_assoc);
         }
     }
+
+    // Free the heap array allocated by decode_em_sta_link_metrics_object.
+    if (params->em_sta_link_metrics_rsp.per_sta_metrics != NULL) {
+        free(params->em_sta_link_metrics_rsp.per_sta_metrics);
+        params->em_sta_link_metrics_rsp.per_sta_metrics = NULL;
+    }
+
     return webconfig_error_none;
 }
 
@@ -1611,17 +1622,17 @@ webconfig_error_t translate_ap_metrics_report_to_easy_mesh_bss_info(webconfig_su
     proto = (webconfig_external_easymesh_t *)data->u.decoded.external_protos;
     if (proto == NULL) {
         wifi_util_error_print(WIFI_WEBCONFIG,"%s:%d: external_protos is NULL\n", __func__, __LINE__);
-        return webconfig_error_translate_to_easymesh;
+        goto cleanup_ap_metrics;
     }
 
     if (proto->get_radio_info(proto->data_model, 0) == NULL) {
         wifi_util_error_print(WIFI_WEBCONFIG,"%s:%d: get_radio_info is NULL\n", __func__, __LINE__);
-        return webconfig_error_translate_to_easymesh;
+        goto cleanup_ap_metrics;
     }
 
     if ((decoded_params->num_radios < MIN_NUM_RADIOS) || (decoded_params->num_radios > MAX_NUM_RADIOS )){
         wifi_util_error_print(WIFI_WEBCONFIG, "%s:%d: Invalid number of radios : %x\n", __func__, __LINE__
, decoded_params->num_radios);
-        return webconfig_error_translate_to_easymesh;
+        goto cleanup_ap_metrics;
     }
 
     em_ap_report = &decoded_params->em_ap_metrics_report;
@@ -1752,7 +1763,27 @@ webconfig_error_t translate_ap_metrics_report_to_easy_mesh_bss_info(webconfig_su
         }
     }
 
-    return webconfig_error_none;
+cleanup_ap_metrics:
+    // Free heap allocations made by decode_em_ap_metrics_report_object for
+    // sta_traffic_stats and sta_link_metrics <E2><80><94> these are only needed during
+    // translation and must be freed here once consumed, including on error paths.
+    if (em_ap_report != NULL) {
+        for (unsigned int i = 0; i < em_ap_report->radio_count; i++) {
+            for (int k = 0; k < MAX_NUM_VAP_PER_RADIO; k++) {
+                em_vap_metrics_t *vap_m = &em_ap_report->radio_reports[i].vap_reports[k];
+                if (vap_m->sta_traffic_stats != NULL) {
+                    free(vap_m->sta_traffic_stats);
+                    vap_m->sta_traffic_stats = NULL;
+                }
+                if (vap_m->sta_link_metrics != NULL) {
+                    free(vap_m->sta_link_metrics);
+                    vap_m->sta_link_metrics = NULL;
+                }
+            }
+        }
+    }
+
+    return (em_ap_report != NULL) ? webconfig_error_none : webconfig_error_translate_to_easymesh;
 }
 #endif

@Nikita-Hakai Nikita-Hakai Jul 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If its urgent to be merged, we can bring this change as a seperate PR too!

#endif
webconfig_data_free(&webconfig_easymesh_data);
return webconfig_error_none;
}
Expand Down
32 changes: 32 additions & 0 deletions source/webconfig/wifi_webconfig_em_ap_metrics_report.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,25 @@ webconfig_error_t encode_em_ap_metrics_report_subdoc(webconfig_t *config, webcon
return webconfig_error_none;
}

// frees the per-vap arrays allocated by decode_em_ap_metrics_report_object
static void free_em_ap_metrics_report_params(webconfig_subdoc_decoded_data_t *params)
{
for (int r = 0; r < MAX_NUM_RADIOS; r++) {
for (int v = 0; v < MAX_NUM_VAP_PER_RADIO; v++) {
em_vap_metrics_t *vap_report = &params->em_ap_metrics_report.radio_reports[r].vap_reports[v];

if (vap_report->sta_traffic_stats != NULL) {
free(vap_report->sta_traffic_stats);
vap_report->sta_traffic_stats = NULL;
}
if (vap_report->sta_link_metrics != NULL) {
free(vap_report->sta_link_metrics);
vap_report->sta_link_metrics = NULL;
}
}
}
}

webconfig_error_t decode_em_ap_metrics_report_subdoc(webconfig_t *config, webconfig_subdoc_data_t *data)
{
webconfig_subdoc_decoded_data_t *params;
Expand Down Expand Up @@ -174,6 +193,14 @@ webconfig_error_t decode_em_ap_metrics_report_subdoc(webconfig_t *config, webcon
if (em_ap_report_arr == NULL || !cJSON_IsArray(em_ap_report_arr)) {
wifi_util_error_print(WIFI_WEBCONFIG, "%s:%d: Invalid or missing EMAPMetricsReport\n",
__func__, __LINE__);
cJSON_Delete(json);
return webconfig_error_decode;
}

if (cJSON_GetArraySize(em_ap_report_arr) > MAX_NUM_RADIOS) {
wifi_util_error_print(WIFI_WEBCONFIG, "%s:%d: EMAPMetricsReport has %d entries, max is %d\n",
__func__, __LINE__, cJSON_GetArraySize(em_ap_report_arr), MAX_NUM_RADIOS);
cJSON_Delete(json);
return webconfig_error_decode;
}

Expand All @@ -182,17 +209,22 @@ webconfig_error_t decode_em_ap_metrics_report_subdoc(webconfig_t *config, webcon
if (em_ap_report_obj == NULL) {
wifi_util_error_print(WIFI_WEBCONFIG, "%s:%d: Invalid EMAPMetricsReport object at index %d\n",
__func__, __LINE__, i);
free_em_ap_metrics_report_params(params);
cJSON_Delete(json);
return webconfig_error_decode;
}

if(decode_em_ap_metrics_report_object(em_ap_report_obj, &params->em_ap_metrics_report.radio_reports[i]) != webconfig_error_none) {
wifi_util_error_print(WIFI_WEBCONFIG, "%s:%d: decode_em_ap_metrics_report_object failed\n", __func__, __LINE__);
free_em_ap_metrics_report_params(params);
cJSON_Delete(json);
return webconfig_error_decode;
}
}
params->em_ap_metrics_report.radio_count = i;

// the subdoc decoder owns u.encoded.json (see decode_radio_subdoc)
cJSON_Delete(json);
return webconfig_error_none;
Comment thread
dkyncu marked this conversation as resolved.
}
#endif
Loading