Merge pull request #299 from nuclearcat/fix-use-after-free

radius: Fix use after free and more

3 files changed, 33 insertions, 15 deletions

diff --git a/accel-pppd/radius/radius_p.h b/accel-pppd/radius/radius_p.h
index 54effa41..26d877a7 100644
--- a/accel-pppd/radius/radius_p.h
+++ b/accel-pppd/radius/radius_p.h
@@ -211,7 +211,7 @@ struct rad_req_t *rad_req_alloc(struct radius_pd_t *rpd, int code, const char *u
 struct rad_req_t *rad_req_alloc2(struct radius_pd_t *rpd, int code, const char *username, in_addr_t addr, int port);
 struct rad_req_t *rad_req_alloc_empty();
 int rad_req_acct_fill(struct rad_req_t *);
-void rad_req_free(struct rad_req_t *);
+struct rad_server_t *rad_req_free(struct rad_req_t *);
 int rad_req_send(struct rad_req_t *req);
 int __rad_req_send(struct rad_req_t *req, int async);
 int rad_req_read(struct triton_md_handler_t *h);
@@ -244,7 +244,7 @@ int rad_dae_src_check(in_addr_t ipaddr);
 
 struct rad_server_t *rad_server_get(int);
 struct rad_server_t *rad_server_get2(int, in_addr_t, int);
-void rad_server_put(struct rad_server_t *, int);
+struct rad_server_t *rad_server_put(struct rad_server_t *, int);
 int rad_server_req_enter(struct rad_req_t *);
 void rad_server_req_exit(struct rad_req_t *);
 int rad_server_req_cancel(struct rad_req_t *, int full);
diff --git a/accel-pppd/radius/req.c b/accel-pppd/radius/req.c
index e3c41996..20984982 100644
--- a/accel-pppd/radius/req.c
+++ b/accel-pppd/radius/req.c
@@ -255,13 +255,15 @@ int rad_req_acct_fill(struct rad_req_t *req)
 	return 0;
 }
 
-void rad_req_free(struct rad_req_t *req)
+struct rad_server_t *rad_req_free(struct rad_req_t *req)
 {
+	struct rad_server_t *s = NULL;
+
 	assert(!req->active);
 	assert(!req->entry.next);
 
 	if (req->serv)
-		rad_server_put(req->serv, req->type);
+		s = rad_server_put(req->serv, req->type);
 
 	if (req->hnd.tpd)
 		triton_md_unregister_handler(&req->hnd, 1);
@@ -278,6 +280,8 @@ void rad_req_free(struct rad_req_t *req)
 		rad_packet_free(req->reply);
 
 	mempool_free(req);
+
+	return s;
 }
 
 static int make_socket(struct rad_req_t *req)
diff --git a/accel-pppd/radius/serv.c b/accel-pppd/radius/serv.c
index 9954afd5..4d97b076 100644
--- a/accel-pppd/radius/serv.c
+++ b/accel-pppd/radius/serv.c
@@ -93,16 +93,24 @@ struct rad_server_t *rad_server_get2(int type, in_addr_t addr, int port)
 	return __rad_server_get(type, NULL, addr, port);
 }
 
-void rad_server_put(struct rad_server_t *s, int type)
+struct rad_server_t *rad_server_put(struct rad_server_t *s, int type)
 {
+	int do_close = 0;
+	int do_free = 0;
+
 	__sync_sub_and_fetch(&s->client_cnt[type], 1);
 
 	if ((s->need_free || s->need_close) && !s->client_cnt[0] && !s->client_cnt[1]) {
-		if (s->need_close)
+		if (s->need_close) {
+			do_close = 1;
 			triton_context_call(&s->ctx, (triton_event_func)serv_ctx_close, &s->ctx);
-		else
+		} else {
+			do_free = 1;
 			__free_server(s);
-	}
+		}
+	} 
+
+	return (do_free || do_close) ? NULL : s;
 }
 
 static void req_wakeup(struct rad_req_t *req)
@@ -383,13 +391,14 @@ static void acct_on_sent(struct rad_req_t *req, int res)
 
 static void acct_on_recv(struct rad_req_t *req)
 {
-	struct rad_server_t *s = req->serv;
+	struct rad_server_t *s = rad_req_free(req);
 
-	rad_req_free(req);
+	if (!s)
+		return;
 
-	if (req->serv->starting) {
-		req->serv->starting = 0;
-		req->serv->acct_on = 1;
+	if (s->starting) {
+		s->starting = 0;
+		s->acct_on = 1;
 	} else
 		__free_server(s);
 }
@@ -397,12 +406,15 @@ static void acct_on_recv(struct rad_req_t *req)
 static void acct_on_timeout(struct triton_timer_t *t)
 {
 	struct rad_req_t *req = container_of(t, typeof(*req), timeout);
-	struct rad_server_t *s = req->serv;
 
 	log_switch(triton_context_self(), NULL);
 
 	if (req->try++ == conf_max_try) {
-		rad_req_free(req);
+		struct rad_server_t *s = rad_req_free(req);
+
+		if (!s)
+			return;
+
 		if (s->starting)
 			s->starting = 0;
 		else
@@ -416,6 +428,8 @@ static void acct_on_timeout(struct triton_timer_t *t)
 static void send_acct_on(struct rad_server_t *s)
 {
 	struct rad_req_t *req = rad_req_alloc_empty();
+	if (!req)
+		return;
 
 	log_switch(triton_context_self(), NULL);