From d045cc8e4cce4ff6adf0ec4d4c2b132ef78dc8a5 Mon Sep 17 00:00:00 2001 From: Jerry Yang Date: Thu, 29 May 2014 14:32:33 +0800 Subject: [PATCH] RADIUS client: Fix crash issue in radius_client_timer() While iterating through RADIUS messages in radius_client_timer(), one message entry may get flushed by "radius_client_retransmit --> radius_client_handle_send_error --> radius_client_init_auth --> radius_change_server --> radius_client_flush". This could result in freed memory being accessed afterwards. Signed-off-by: Jerry Yang --- src/radius/radius_client.c | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/src/radius/radius_client.c b/src/radius/radius_client.c index 6049b363e..0f00a65c8 100644 --- a/src/radius/radius_client.c +++ b/src/radius/radius_client.c @@ -295,8 +295,12 @@ int radius_client_register(struct radius_client_data *radius, } -static void radius_client_handle_send_error(struct radius_client_data *radius, - int s, RadiusType msg_type) +/* + * Returns >0 if message queue was flushed (i.e., the message that triggered + * the error is not available anymore) + */ +static int radius_client_handle_send_error(struct radius_client_data *radius, + int s, RadiusType msg_type) { #ifndef CONFIG_NATIVE_WINDOWS int _errno = errno; @@ -309,12 +313,18 @@ static void radius_client_handle_send_error(struct radius_client_data *radius, " try to connect again"); eloop_unregister_read_sock(s); close(s); - if (msg_type == RADIUS_ACCT || msg_type == RADIUS_ACCT_INTERIM) + if (msg_type == RADIUS_ACCT || + msg_type == RADIUS_ACCT_INTERIM) { radius_client_init_acct(radius); - else + return 0; + } else { radius_client_init_auth(radius); + return 1; + } } #endif /* CONFIG_NATIVE_WINDOWS */ + + return 0; } @@ -353,8 +363,11 @@ static int radius_client_retransmit(struct radius_client_data *radius, os_get_reltime(&entry->last_attempt); buf = radius_msg_get_buf(entry->msg); - if (send(s, wpabuf_head(buf), wpabuf_len(buf), 0) < 0) - radius_client_handle_send_error(radius, s, entry->msg_type); + if (send(s, wpabuf_head(buf), wpabuf_len(buf), 0) < 0) { + if (radius_client_handle_send_error(radius, s, entry->msg_type) + > 0) + return 0; + } entry->next_try = now + entry->next_wait; entry->next_wait *= 2; @@ -378,6 +391,7 @@ static void radius_client_timer(void *eloop_ctx, void *timeout_ctx) struct radius_msg_list *entry, *prev, *tmp; int auth_failover = 0, acct_failover = 0; char abuf[50]; + size_t prev_num_msgs; entry = radius->msgs; if (!entry) @@ -388,6 +402,7 @@ static void radius_client_timer(void *eloop_ctx, void *timeout_ctx) prev = NULL; while (entry) { + prev_num_msgs = radius->num_msgs; if (now.sec >= entry->next_try && radius_client_retransmit(radius, entry, now.sec)) { if (prev) @@ -402,6 +417,14 @@ static void radius_client_timer(void *eloop_ctx, void *timeout_ctx) continue; } + if (prev_num_msgs != radius->num_msgs) { + wpa_printf(MSG_DEBUG, + "RADIUS: Message removed from queue - restart from beginning"); + entry = radius->msgs; + prev = NULL; + continue; + } + if (entry->attempts > RADIUS_CLIENT_NUM_FAILOVER) { if (entry->msg_type == RADIUS_ACCT || entry->msg_type == RADIUS_ACCT_INTERIM)