diff mbox

Fix crash issue in radius_client_timer()

Message ID 1401345153-7848-1-git-send-email-yxjsolid@gmail.com
State Accepted
Headers show

Commit Message

Jerry Yang May 29, 2014, 6:32 a.m. UTC 
From: Jerry Yang <xyang@sonicwall.com>

While iterate through Radius message in "radius_client_timer",
one message entry may flushed by "radius_client_retransmit -->
radius_client_handle_send_error --> radius_client_init_auth -->
radius_change_server --> radius_client_flush".

Signed-off-by: Jerry Yang <xyang@sonicwall.com>
---
 src/radius/radius_client.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Jouni Malinen May 31, 2014, 9:50 a.m. UTC | #1 
On Thu, May 29, 2014 at 02:32:33PM +0800, Jerry Yang wrote:
> While iterate through Radius message in "radius_client_timer",
> one message entry may flushed by "radius_client_retransmit -->
> radius_client_handle_send_error --> radius_client_init_auth -->
> radius_change_server --> radius_client_flush".

Thanks, applied. While working on better regression test coverage for
this, I found couple of other issues with RADIUS client send() error
handling and server failover. This are now fixed as well and hopefully
that better test coverage will help keep this area in better working
condition from now.
diff mbox

Patch

diff --git a/src/radius/radius_client.c b/src/radius/radius_client.c
index 7625996..96ea12c 100644
--- a/src/radius/radius_client.c
+++ b/src/radius/radius_client.c
@@ -325,6 +325,7 @@  static int radius_client_retransmit(struct radius_client_data *radius,
 	struct hostapd_radius_servers *conf = radius->conf;
 	int s;
 	struct wpabuf *buf;
+	RadiusType msg_type;
 
 	if (entry->msg_type == RADIUS_ACCT ||
 	    entry->msg_type == RADIUS_ACCT_INTERIM) {
@@ -353,9 +354,14 @@  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)
+	if (send(s, wpabuf_head(buf), wpabuf_len(buf), 0) < 0) {
+		msg_type = entry->msg_type;
 		radius_client_handle_send_error(radius, s, entry->msg_type);
 
+		if (msg_type == RADIUS_AUTH)
+			return 0;
+	}
+
 	entry->next_try = now + entry->next_wait;
 	entry->next_wait *= 2;
 	if (entry->next_wait > RADIUS_CLIENT_MAX_WAIT)
@@ -378,6 +384,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 +395,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 +410,12 @@  static void radius_client_timer(void *eloop_ctx, void *timeout_ctx)
 			continue;
 		}
 
+		if (prev_num_msgs != radius->num_msgs) {
+			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)