net: openthread: Fix possible deadlock in net_mgmt handlers

There is a risk of deadlock in case net_if APIs are used from within
net_mgmt handlers as both module APIs are protected with their own
mutexes.

The scenario observed with OpenThread happend when
NET_EVENT_IPV6_ADDR_ADD/NET_EVENT_IPV6_MADDR_ADD events were processed.
The net_mgmt mutex is locked when both, an event handler is being
processed (from a separate net_mgmt thread) and when an event is raised
(for example when a new address is added on an interface). In case a
net_mgmt handler tried to use some mutex-protected net_if API, we could
end up in a deadlock situation - the net_mgmt would wait for the net_if
mutex to release, while some other thread (in this case main during
initialization) could wait within some net_if function, pending on
net_mgmt mutex to be released to notify the event.

Fix this, by preventing net_if APIs from being used from within OT
net_mgmt handlers.

Additionally, simplify the net_mgmt handlers logic, by making use of
additional info provided with an event. Instead of blindy assuming that
recently added address was the last on the list (which might not always
be the case, if addresses are added/removed dynamically), read the
actual address being added from the net_mgmt_event_callback structure.

Signed-off-by: Robert Lubos <[email protected]>

Author: rlubos

subsys/net/l2/openthread/openthread.c

@@ -122,10 +122,14 @@ static void ipv6_addr_event_handler(struct net_mgmt_event_callback *cb,
 		return;
 	}
 
+	if (cb->info == NULL || cb->info_length != sizeof(struct in6_addr)) {
+		return;
+	}
+
 	if (mgmt_event == NET_EVENT_IPV6_ADDR_ADD) {
-		add_ipv6_addr_to_ot(ot_context);
+		add_ipv6_addr_to_ot(ot_context, (const struct in6_addr *)cb->info);
 	} else if (mgmt_event == NET_EVENT_IPV6_MADDR_ADD) {
-		add_ipv6_maddr_to_ot(ot_context);
+		add_ipv6_maddr_to_ot(ot_context, (const struct in6_addr *)cb->info);
 	}
 }
 #endif

subsys/net/l2/openthread/openthread_utils.c

@@ -157,42 +157,52 @@ void add_ipv6_addr_to_zephyr(struct openthread_context *context)
 	}
 }
 
-void add_ipv6_addr_to_ot(struct openthread_context *context)
+void add_ipv6_addr_to_ot(struct openthread_context *context,
+			 const struct in6_addr *addr6)
 {
-	struct net_if *iface = context->iface;
-	struct otNetifAddress addr;
+	struct otNetifAddress addr = { 0 };
 	struct net_if_ipv6 *ipv6;
+	struct net_if_addr *if_addr = NULL;
 	int i;
 
-	(void)memset(&addr, 0, sizeof(addr));
-
-	if (net_if_config_ipv6_get(iface, &ipv6) < 0) {
-		NET_DBG("Cannot allocate IPv6 address");
+	/* IPv6 struct should've already been allocated when we get an
+	 * address added event.
+	 */
+	ipv6 = context->iface->config.ip.ipv6;
+	if (ipv6 == NULL) {
+		NET_ERR("No IPv6 container allocated");
 		return;
 	}
 
-	/* save the last added IP address for this interface */
+	/* Find the net_if_addr structure containing the newly added address. */
 	for (i = NET_IF_MAX_IPV6_ADDR - 1; i >= 0; i--) {
-		if (ipv6->unicast[i].is_used) {
-			memcpy(&addr.mAddress,
-			       &ipv6->unicast[i].address.in6_addr,
-			       sizeof(addr.mAddress));
+		if (ipv6->unicast[i].is_used &&
+		    net_ipv6_addr_cmp(&ipv6->unicast[i].address.in6_addr,
+				      addr6)) {
+			if_addr = &ipv6->unicast[i];
 			break;
 		}
 	}
 
-	ipv6->unicast[i].is_mesh_local = is_mesh_local(
+	if (if_addr == NULL) {
+		NET_ERR("No corresponding net_if_addr found");
+		return;
+	}
+
+	memcpy(&addr.mAddress, addr6, sizeof(addr.mAddress));
+
+	if_addr->is_mesh_local = is_mesh_local(
 			context, ipv6->unicast[i].address.in6_addr.s6_addr);
 
 	addr.mValid = true;
 	addr.mPreferred = true;
 	addr.mPrefixLength = 64;
 
-	if (ipv6->unicast[i].addr_type == NET_ADDR_AUTOCONF) {
+	if (if_addr->addr_type == NET_ADDR_AUTOCONF) {
 		addr.mAddressOrigin = OT_ADDRESS_ORIGIN_SLAAC;
-	} else if (ipv6->unicast[i].addr_type == NET_ADDR_DHCP) {
+	} else if (if_addr->addr_type == NET_ADDR_DHCP) {
 		addr.mAddressOrigin = OT_ADDRESS_ORIGIN_DHCPV6;
-	} else if (ipv6->unicast[i].addr_type == NET_ADDR_MANUAL) {
+	} else if (if_addr->addr_type == NET_ADDR_MANUAL) {
 		addr.mAddressOrigin = OT_ADDRESS_ORIGIN_MANUAL;
 	} else {
 		NET_ERR("Unknown address type");
@@ -213,26 +223,12 @@ void add_ipv6_addr_to_ot(struct openthread_context *context)
 	}
 }
 
-void add_ipv6_maddr_to_ot(struct openthread_context *context)
+void add_ipv6_maddr_to_ot(struct openthread_context *context,
+			  const struct in6_addr *addr6)
 {
 	struct otIp6Address addr;
-	struct net_if_ipv6 *ipv6;
-	int i;
 
-	if (net_if_config_ipv6_get(context->iface, &ipv6) < 0) {
-		NET_DBG("Cannot allocate IPv6 address");
-		return;
-	}
-
-	/* save the last added IP address for this interface */
-	for (i = NET_IF_MAX_IPV6_MADDR - 1; i >= 0; i--) {
-		if (ipv6->mcast[i].is_used) {
-			memcpy(&addr,
-			       &ipv6->mcast[i].address.in6_addr,
-			       sizeof(addr));
-			break;
-		}
-	}
+	memcpy(&addr, addr6, sizeof(addr));
 
 	openthread_api_mutex_lock(context);
 	otIp6SubscribeMulticastAddress(context->instance, &addr);

subsys/net/l2/openthread/openthread_utils.h

@@ -20,8 +20,10 @@ void dump_pkt(const char *str, struct net_pkt *pkt);
 #endif
 
 void add_ipv6_addr_to_zephyr(struct openthread_context *context);
-void add_ipv6_addr_to_ot(struct openthread_context *context);
-void add_ipv6_maddr_to_ot(struct openthread_context *context);
+void add_ipv6_addr_to_ot(struct openthread_context *context,
+			 const struct in6_addr *addr6);
+void add_ipv6_maddr_to_ot(struct openthread_context *context,
+			  const struct in6_addr *addr6);
 void add_ipv6_maddr_to_zephyr(struct openthread_context *context);
 void rm_ipv6_addr_from_zephyr(struct openthread_context *context);
 void rm_ipv6_maddr_from_zephyr(struct openthread_context *context);