kmod-oaf: avoid af_client use after free

jjm2473 · Mar 14, 2024 · 677cbed · 677cbed
1 parent 1d089f0
commit 677cbed
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 25 deletions.
diff --git a/oaf/src/app_filter.c b/oaf/src/app_filter.c
@@ -781,10 +781,9 @@ int af_match_one(flow_info_t *flow, af_feature_node_t *node)
 	return ret;
 }
 
-int app_filter_match(flow_info_t *flow)
+int app_filter_match(flow_info_t *flow, unsigned char *mac)
 {
 	af_feature_node_t *n, *node;
-	af_client_info_t *client = NULL;
 	feature_list_read_lock();
 	if (!list_empty(&af_feature_head))
 	{
@@ -794,12 +793,8 @@ int app_filter_match(flow_info_t *flow)
 			{
 				flow->app_id = node->app_id;
 				strncpy(flow->app_name, node->app_name, sizeof(flow->app_name) - 1);
-				client = find_af_client_by_ip(flow->src);
-				if (!client)
-				{
-					goto EXIT;
-				}
-				if (is_user_match_enable() && !find_af_mac(client->mac))
+
+				if (is_user_match_enable() && !find_af_mac(mac))
 				{
 					goto EXIT;
 				}
@@ -842,21 +837,29 @@ static int af_get_visit_index(af_client_info_t *node, int app_id)
 	return 0;
 }
 
-int af_update_client_app_info(af_client_info_t *node, int app_id, int drop)
+int af_update_client_app_info(unsigned char *mac, int app_id, int drop)
 {
+	af_client_info_t *node;
 	int index = -1;
-	if (!node)
+	AF_CLIENT_LOCK_W();
+	node = find_af_client(mac);
+	if (!node) {
+		AF_CLIENT_UNLOCK_W();
 		return -1;
+	}
 
 	index = af_get_visit_index(node, app_id);
-	if (index < 0 || index >= MAX_RECORD_APP_NUM)
+	if (index < 0 || index >= MAX_RECORD_APP_NUM) {
+		AF_CLIENT_UNLOCK_W();
 		return 0;
+	}
 	node->visit_info[index].total_num++;
 	if (drop)
 		node->visit_info[index].drop_num++;
 	node->visit_info[index].app_id = app_id;
 	node->visit_info[index].latest_time = af_get_timestamp_sec();
 	node->visit_info[index].latest_action = drop;
+	AF_CLIENT_UNLOCK_W();
 	return 0;
 }
 
@@ -952,15 +955,15 @@ u_int32_t app_filter_hook_bypass_handle(struct sk_buff *skb, struct net_device *
 		return NF_ACCEPT;
 	}
 	client->update_jiffies = jiffies;
+	client->ip = flow.src;
 	AF_CLIENT_UNLOCK_W();
 
 	if (0 != dpi_main(skb, &flow))
 		return NF_ACCEPT;
 
-	client->ip = flow.src;
-	app_filter_match(&flow);
+	app_filter_match(&flow, smac);
 	if (flow.app_id != 0){
-		af_update_client_app_info(client, flow.app_id, flow.drop);
+		af_update_client_app_info(smac, flow.app_id, flow.drop);
 	}
 	if (flow.drop)
 	{
@@ -1002,6 +1005,7 @@ u_int32_t app_filter_hook_gateway_handle(struct sk_buff *skb, struct net_device
 		return NF_ACCEPT;
 	}
 	client->update_jiffies = jiffies;
+	memcpy(smac, client->mac, ETH_ALEN);
 	AF_CLIENT_UNLOCK_R();
 
 	if ((ct->mark & NF_MARK_BIT) != 0)
@@ -1010,9 +1014,7 @@ u_int32_t app_filter_hook_gateway_handle(struct sk_buff *skb, struct net_device
 		if (app_id > 1000 && app_id < 9999){
 			if (NF_DROP_BIT == (ct->mark & NF_DROP_BIT))
 				drop = 1;
-			AF_CLIENT_LOCK_W();
-			af_update_client_app_info(client, app_id, drop);
-			AF_CLIENT_UNLOCK_W();
+			af_update_client_app_info(smac, app_id, drop);
 
 			if (drop){
 				return NF_DROP;
@@ -1034,14 +1036,12 @@ u_int32_t app_filter_hook_gateway_handle(struct sk_buff *skb, struct net_device
 	if (0 != dpi_main(skb, &flow))
 		return NF_ACCEPT;
 
-	app_filter_match(&flow);
+	app_filter_match(&flow, smac);
 
 	if (flow.app_id != 0)
 	{
 		ct->mark = (ct->mark & (~NF_APP_BIT)) | (flow.app_id << 16) | NF_MARK_BIT;
-		AF_CLIENT_LOCK_W();
-		af_update_client_app_info(client, flow.app_id, flow.drop);
-		AF_CLIENT_UNLOCK_W();
+		af_update_client_app_info(smac, flow.app_id, flow.drop);
 		AF_LMT_INFO("match %s %pI4(%d)--> %pI4(%d) len = %d, %d\n ", IPPROTO_TCP == flow.l4_protocol ? "tcp" : "udp",
 					&flow.src, flow.sport, &flow.dst, flow.dport, skb->len, flow.app_id);
 		if (flow.drop)

diff --git a/oaf/src/app_filter.h b/oaf/src/app_filter.h
@@ -165,7 +165,7 @@ int af_get_app_status(int appid);
 int regexp_match(char *reg, char *text);
 void af_mac_list_init(void);
 void af_mac_list_clear(void);
-af_mac_info_t * find_af_mac(unsigned char *mac);
+int find_af_mac(unsigned char *mac);
 int is_user_match_enable(void);
 extern int g_oaf_enable;
 

diff --git a/oaf/src/app_filter_config.c b/oaf/src/app_filter_config.c
@@ -158,21 +158,24 @@ int hash_mac(unsigned char *mac)
 		return mac[5] & (MAX_AF_MAC_HASH_SIZE - 1);
 }
 
-af_mac_info_t *find_af_mac(unsigned char *mac)
+int find_af_mac(unsigned char *mac)
 {
 	af_mac_info_t *node;
 	unsigned int index;
 
 	index = hash_mac(mac);
+	AF_MAC_LOCK_R();
 	list_for_each_entry(node, &af_mac_list_table[index], hlist)
 	{
 		if (0 == memcmp(node->mac, mac, 6))
 		{
 			AF_DEBUG("match mac:" MAC_FMT "\n", MAC_ARRAY(node->mac));
-			return node;
+			AF_MAC_UNLOCK_R();
+			return 1;
 		}
 	}
-	return NULL;
+	AF_MAC_UNLOCK_R();
+	return 0;
 }
 
 static af_mac_info_t *
@@ -194,8 +197,10 @@ af_mac_add(unsigned char *mac)
 	index = hash_mac(mac);
 
 	AF_LMT_INFO("new client mac=" MAC_FMT "\n", MAC_ARRAY(node->mac));
+	AF_MAC_LOCK_W();
 	total_mac++;
 	list_add(&(node->hlist), &af_mac_list_table[index]);
+	AF_MAC_UNLOCK_W();
 	return node;
 }