From 41baee12f9f958266512f4a3eb46e310442ad017 Mon Sep 17 00:00:00 2001 From: emanuele-f Date: Sun, 26 Jun 2022 23:33:40 +0200 Subject: [PATCH] Improve handling of error cases to prevent leaks --- app/src/main/jni/core/capture_root.c | 8 +++----- app/src/main/jni/core/capture_vpn.c | 5 ++++- app/src/main/jni/core/pcapdroid.c | 26 ++++++++++++++++---------- app/src/main/jni/core/pcapdroid.h | 2 +- 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/app/src/main/jni/core/capture_root.c b/app/src/main/jni/core/capture_root.c index 4bfb557e..437d3511 100644 --- a/app/src/main/jni/core/capture_root.c +++ b/app/src/main/jni/core/capture_root.c @@ -431,11 +431,9 @@ static void purge_expired_connections(pcapdroid_t *pd, uint8_t purge_all) { conn->data->update_type |= CONN_UPDATE_STATS; } - if(conn->data->update_type != 0) { - // The connection data cannot be free now as it is enqueued in a conn_array_t. - // It will be free in sendConnectionsDump. - pd_notify_connection_update(pd, &conn->tuple, conn->data); - } else { + // If there is a pending notification, the connection data cannot be free now as it is enqueued in a conn_array_t + if((conn->data->update_type == 0) || (pd_notify_connection_update(pd, &conn->tuple, conn->data) < 0)) { + // no pending notification/pd_notify_connection_update failed, free now pd_purge_connection(pd, conn->data); conn->data = NULL; } diff --git a/app/src/main/jni/core/capture_vpn.c b/app/src/main/jni/core/capture_vpn.c index b2396cf5..10018a2e 100644 --- a/app/src/main/jni/core/capture_vpn.c +++ b/app/src/main/jni/core/capture_vpn.c @@ -327,7 +327,10 @@ static void connection_closed(zdtun_t *zdt, const zdtun_conn_t *conn_info) { // Send last notification // Will free the data in sendConnectionsDump data->update_type |= CONN_UPDATE_STATS; - pd_notify_connection_update(pd, tuple, data); + if(pd_notify_connection_update(pd, tuple, data) < 0) { + pd_purge_connection(pd, data); + return; + } pd_giveup_dpi(pd, data, tuple); data->status = zdtun_conn_get_status(conn_info); diff --git a/app/src/main/jni/core/pcapdroid.c b/app/src/main/jni/core/pcapdroid.c index 42d6c2f4..12c38c75 100644 --- a/app/src/main/jni/core/pcapdroid.c +++ b/app/src/main/jni/core/pcapdroid.c @@ -114,14 +114,14 @@ void pd_purge_connection(pcapdroid_t *pd, pd_conn_t *data) { /* ******************************************************* */ -static void notif_connection(pcapdroid_t *pd, conn_array_t *arr, const zdtun_5tuple_t *tuple, pd_conn_t *data) { +static int notif_connection(pcapdroid_t *pd, conn_array_t *arr, const zdtun_5tuple_t *tuple, pd_conn_t *data) { // End the detection when the connection is closed // Always check this, even pending_notification are present if(data->status >= CONN_STATUS_CLOSED) pd_giveup_dpi(pd, data, tuple); if(data->pending_notification) - return; + return 0; if(arr->cur_items >= arr->size) { /* Extend array */ @@ -130,7 +130,7 @@ static void notif_connection(pcapdroid_t *pd, conn_array_t *arr, const zdtun_5tu if(arr->items == NULL) { log_e("realloc(conn_array_t) (%d items) failed", arr->size); - return; + return -1; } } @@ -138,12 +138,14 @@ static void notif_connection(pcapdroid_t *pd, conn_array_t *arr, const zdtun_5tu slot->tuple = *tuple; slot->data = data; data->pending_notification = true; + return 0; } -/* Call this when the connection data has changed. The connection data will sent to JAVA during the - * next sendConnectionsDump. The type of change is determined by the data->update_type. */ -void pd_notify_connection_update(pcapdroid_t *pd, const zdtun_5tuple_t *tuple, pd_conn_t *data) { - notif_connection(pd, &pd->conns_updates, tuple, data); +/* Call this when the connection data has changed. The connection data will be sent to JAVA during the + * next sendConnectionsDump. The type of change is determined by the data->update_type. + * A negative value is returned if the connection update could not be enqueued. */ +int pd_notify_connection_update(pcapdroid_t *pd, const zdtun_5tuple_t *tuple, pd_conn_t *data) { + return notif_connection(pd, &pd->conns_updates, tuple, data); } /* ******************************************************* */ @@ -319,7 +321,13 @@ pd_conn_t* pd_new_connection(pcapdroid_t *pd, const zdtun_5tuple_t *tuple, int u /* nDPI */ if((data->ndpi_flow = ndpi_calloc(1, SIZEOF_FLOW_STRUCT)) == NULL) { log_e("ndpi_flow_malloc failed"); - conn_free_ndpi(data); + pd_purge_connection(pd, data); + return(NULL); + } + + if(notif_connection(pd, &pd->new_conns, tuple, data) < 0) { + pd_purge_connection(pd, data); + return(NULL); } data->uid = uid; @@ -431,8 +439,6 @@ pd_conn_t* pd_new_connection(pcapdroid_t *pd, const zdtun_5tuple_t *tuple, int u fw_num_checked_connections++; } - notif_connection(pd, &pd->new_conns, tuple, data); - return(data); } diff --git a/app/src/main/jni/core/pcapdroid.h b/app/src/main/jni/core/pcapdroid.h index 4b947eb0..02ae7bae 100644 --- a/app/src/main/jni/core/pcapdroid.h +++ b/app/src/main/jni/core/pcapdroid.h @@ -366,7 +366,7 @@ void pd_dump_packet(pcapdroid_t *pd, const char *pktbuf, int pktlen, const struc void pd_housekeeping(pcapdroid_t *pd); pd_conn_t* pd_new_connection(pcapdroid_t *pd, const zdtun_5tuple_t *tuple, int uid); void pd_purge_connection(pcapdroid_t *pd, pd_conn_t *data); -void pd_notify_connection_update(pcapdroid_t *pd, const zdtun_5tuple_t *tuple, pd_conn_t *data); +int pd_notify_connection_update(pcapdroid_t *pd, const zdtun_5tuple_t *tuple, pd_conn_t *data); void pd_giveup_dpi(pcapdroid_t *pd, pd_conn_t *data, const zdtun_5tuple_t *tuple); const char* pd_get_proto_name(pcapdroid_t *pd, uint16_t proto, uint16_t alpn, int ipproto);