service: check number of connected services before changing rp_filter

Commit Message

Ram Subramanian Dec. 10, 2024, 10:56 p.m. UTC

service_rp_filter() keeps track of the number of connected services via
a global variable `connected_networks_count` that is incremented when a
service is connected, and decremented when a service is disconnected.
However, it does not ensure that a service was previously connected
before decrementing the counter.

This can cause `connected_networks_count` to go out of sync with the
read number. For example, if a service fails association, it transitions
directly from associating -> disconnected. Though it didn't increment
`connected_networks_count` it does end up decrementing it.
---
 src/service.c | 51 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 35 insertions(+), 16 deletions(-)

Patch

diff --git a/src/service.c b/src/service.c
index 805cfca7..fcdfd78e 100644
--- a/src/service.c
+++ b/src/service.c
@@ -8408,6 +8408,29 @@  static size_t service_get_count(void)
 	return service_list ? g_list_length(service_list) : 0;
 }
 
+/**
+ *  @brief
+ *    Calculates the number of services in 'ready' or 'online' state
+ *
+ *  @returns
+ *    Service count.
+ */
+static size_t num_connected_services(void)
+{
+        size_t result = 0;
+        GList *list;
+        struct connman_service *service;
+
+        for (list = service_list; list; list = list->next) {
+                service = list->data;
+
+                if (is_connected(service->state))
+                        result++;
+        }
+
+        return result;
+}
+
 /**
  *  @brief
  *    Get the route metric/priority for the specified service.
@@ -9314,12 +9337,13 @@  enum connman_service_state __connman_service_ipconfig_get_state(
  * connectivity works ok. This is only done for IPv4 networks as IPv6
  * does not have rp_filter knob.
  */
-static int connected_networks_count;
+static int rp_filter_serviced_at = 0;
 static int original_rp_filter;
 
 static void service_rp_filter(struct connman_service *service,
 				bool connected)
 {
+	int num_connected;
 	enum connman_ipconfig_method method;
 
 	method = __connman_ipconfig_get_method(service->ipconfig_ipv4);
@@ -9335,30 +9359,25 @@  static void service_rp_filter(struct connman_service *service,
 		break;
 	}
 
-	if (connected) {
-		if (connected_networks_count == 1) {
-			int filter_value;
-			filter_value = __connman_ipconfig_set_rp_filter();
-			if (filter_value < 0)
+	num_connected = num_connected_services();
+	if (num_connected != rp_filter_serviced_at) {
+		if (num_connected > 1) {
+			int rc = __connman_ipconfig_set_rp_filter();
+			if (rc < 0)
 				return;
 
-			original_rp_filter = filter_value;
-		}
-		connected_networks_count++;
-
-	} else {
-		if (connected_networks_count == 2)
+			original_rp_filter = rc;
+		} else if (rp_filter_serviced_at > 1) {
 			__connman_ipconfig_unset_rp_filter(original_rp_filter);
+		}
 
-		connected_networks_count--;
-		if (connected_networks_count < 0)
-			connected_networks_count = 0;
+		rp_filter_serviced_at = num_connected;
 	}
 
 	DBG("%s %s ipconfig %p method %d count %d filter %d",
 		connected ? "connected" : "disconnected", service->identifier,
 		service->ipconfig_ipv4, method,
-		connected_networks_count, original_rp_filter);
+		num_connected, original_rp_filter);
 }
 
 int __connman_service_ipconfig_indicate_state(struct connman_service *service,