net: if: Release the interface lock early when rejoining mcast groups

In order to avoid any mutex deadlocks between iface->lock and
TX lock, release the interface lock before calling a function
that will acquire TX lock. See previous commit for similar issue
in RS timer handling. So here we create a separate list of multicast
addresses that are to be rejoined when network interface comes up
and then rejoin the groups without iface->lock held.

Signed-off-by: Jukka Rissanen <[email protected]>

Author: jukkar

include/zephyr/net/net_if.h

@@ -158,6 +158,9 @@ struct net_if_mcast_addr {
 	/** IP address */
 	struct net_addr address;
 
+	/** Rejoining multicast groups list node */
+	sys_snode_t rejoin_node;
+
 #if defined(CONFIG_NET_IPV4_IGMPV3)
 	/** Sources to filter on */
 	struct net_addr sources[CONFIG_NET_IF_MCAST_IPV4_SOURCE_COUNT];

subsys/net/ip/net_if.c

@@ -1641,7 +1641,9 @@ void net_if_nbr_reachability_hint(struct net_if *iface, const struct in6_addr *i
  */
 static void rejoin_ipv6_mcast_groups(struct net_if *iface)
 {
+	struct net_if_mcast_addr *ifaddr, *next;
 	struct net_if_ipv6 *ipv6;
+	sys_slist_t rejoin_needed;
 
 	net_if_lock(iface);
 
@@ -1663,27 +1665,41 @@ static void rejoin_ipv6_mcast_groups(struct net_if *iface)
 		join_mcast_nodes(iface, &ipv6->unicast[i].address.in6_addr);
 	}
 
+	sys_slist_init(&rejoin_needed);
+
 	/* Rejoin any mcast address present on the interface, but marked as not joined. */
 	ARRAY_FOR_EACH(ipv6->mcast, i) {
-		int ret;
-
 		if (!ipv6->mcast[i].is_used ||
 		    net_if_ipv6_maddr_is_joined(&ipv6->mcast[i])) {
 			continue;
 		}
 
-		ret = net_ipv6_mld_join(iface, &ipv6->mcast[i].address.in6_addr);
+		sys_slist_prepend(&rejoin_needed, &ipv6->mcast[i].rejoin_node);
+	}
+
+	net_if_unlock(iface);
+
+	/* Start DAD for all the addresses without holding the iface lock
+	 * to avoid any possible mutex deadlock issues.
+	 */
+	SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&rejoin_needed,
+					  ifaddr, next, rejoin_node) {
+		int ret;
+
+		ret = net_ipv6_mld_join(iface, &ifaddr->address.in6_addr);
 		if (ret < 0) {
 			NET_ERR("Cannot join mcast address %s for %d (%d)",
-				net_sprint_ipv6_addr(&ipv6->mcast[i].address.in6_addr),
+				net_sprint_ipv6_addr(&ifaddr->address.in6_addr),
 				net_if_get_by_iface(iface), ret);
 		} else {
 			NET_DBG("Rejoined mcast address %s for %d",
-				net_sprint_ipv6_addr(&ipv6->mcast[i].address.in6_addr),
+				net_sprint_ipv6_addr(&ifaddr->address.in6_addr),
 				net_if_get_by_iface(iface));
 		}
 	}
 
+	return;
+
 out:
 	net_if_unlock(iface);
 }
@@ -2024,18 +2040,7 @@ struct net_if_addr *net_if_ipv6_addr_add(struct net_if *iface,
 		if (!(l2_flags_get(iface) & NET_L2_POINT_TO_POINT) &&
 		    !net_ipv6_is_addr_loopback(addr) &&
 		    !net_if_flag_is_set(iface, NET_IF_IPV6_NO_ND)) {
-			/* RFC 4862 5.4.2
-			 * Before sending a Neighbor Solicitation, an interface
-			 * MUST join the all-nodes multicast address and the
-			 * solicited-node multicast address of the tentative
-			 * address.
-			 */
-			/* The allnodes multicast group is only joined once as
-			 * net_ipv6_mld_join() checks if we have already
-			 * joined.
-			 */
-			join_mcast_nodes(iface,
-					 &ipv6->unicast[i].address.in6_addr);
+			/* The groups are joined without locks held */
 			do_dad = true;
 		} else {
 			/* If DAD is not done for point-to-point links, then
@@ -2056,6 +2061,18 @@ struct net_if_addr *net_if_ipv6_addr_add(struct net_if *iface,
 	net_if_unlock(iface);
 
 	if (ifaddr != NULL && do_dad) {
+		/* RFC 4862 5.4.2
+		 * Before sending a Neighbor Solicitation, an interface
+		 * MUST join the all-nodes multicast address and the
+		 * solicited-node multicast address of the tentative
+		 * address.
+		 */
+		/* The allnodes multicast group is only joined once as
+		 * net_ipv6_mld_join() checks if we have already
+		 * joined.
+		 */
+		join_mcast_nodes(iface, &ifaddr->address.in6_addr);
+
 		net_if_ipv6_start_dad(iface, ifaddr);
 	}
 
@@ -4912,7 +4929,9 @@ static void iface_ipv4_start(struct net_if *iface)
  */
 static void rejoin_ipv4_mcast_groups(struct net_if *iface)
 {
+	struct net_if_mcast_addr *ifaddr, *next;
 	struct net_if_ipv4 *ipv4;
+	sys_slist_t rejoin_needed;
 
 	net_if_lock(iface);
 
@@ -4924,27 +4943,37 @@ static void rejoin_ipv4_mcast_groups(struct net_if *iface)
 		goto out;
 	}
 
+	sys_slist_init(&rejoin_needed);
+
 	/* Rejoin any mcast address present on the interface, but marked as not joined. */
 	ARRAY_FOR_EACH(ipv4->mcast, i) {
-		int ret;
-
 		if (!ipv4->mcast[i].is_used ||
 		    net_if_ipv4_maddr_is_joined(&ipv4->mcast[i])) {
 			continue;
 		}
 
-		ret = net_ipv4_igmp_join(iface, &ipv4->mcast[i].address.in_addr, NULL);
+		sys_slist_prepend(&rejoin_needed, &ipv4->mcast[i].rejoin_node);
+	}
+
+	net_if_unlock(iface);
+
+	SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&rejoin_needed, ifaddr, next, rejoin_node) {
+		int ret;
+
+		ret = net_ipv4_igmp_join(iface, &ifaddr->address.in_addr, NULL);
 		if (ret < 0) {
 			NET_ERR("Cannot join mcast address %s for %d (%d)",
-				net_sprint_ipv4_addr(&ipv4->mcast[i].address.in_addr),
+				net_sprint_ipv4_addr(&ifaddr->address.in_addr),
 				net_if_get_by_iface(iface), ret);
 		} else {
 			NET_DBG("Rejoined mcast address %s for %d",
-				net_sprint_ipv4_addr(&ipv4->mcast[i].address.in_addr),
+				net_sprint_ipv4_addr(&ifaddr->address.in_addr),
 				net_if_get_by_iface(iface));
 		}
 	}
 
+	return;
+
 out:
 	net_if_unlock(iface);
 }