From 1cb7a3adb57e0a9534e5d0bfc93d18661cacb6a4 Mon Sep 17 00:00:00 2001 From: Robert Lubos Date: Fri, 13 Sep 2024 15:04:30 +0200 Subject: [PATCH 1/2] net: iface: Add missing interface mutex locks net_if_ipv4/6_addr_rm() were missing the iface mutex lock, this commit adds it. Signed-off-by: Robert Lubos --- subsys/net/ip/net_if.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/subsys/net/ip/net_if.c b/subsys/net/ip/net_if.c index bb569798707c..8e433ddcba56 100644 --- a/subsys/net/ip/net_if.c +++ b/subsys/net/ip/net_if.c @@ -1964,27 +1964,34 @@ struct net_if_addr *net_if_ipv6_addr_add(struct net_if *iface, bool net_if_ipv6_addr_rm(struct net_if *iface, const struct in6_addr *addr) { struct net_if_ipv6 *ipv6; + bool result = true; int ret; NET_ASSERT(addr); + net_if_lock(iface); + ipv6 = iface->config.ip.ipv6; if (!ipv6) { - return false; + result = false; + goto out; } ret = net_if_addr_unref(iface, AF_INET6, addr); if (ret > 0) { NET_DBG("Address %s still in use (ref %d)", net_sprint_ipv6_addr(addr), ret); - return false; - + result = false; + goto out; } else if (ret < 0) { NET_DBG("Address %s not found (%d)", net_sprint_ipv6_addr(addr), ret); } - return true; +out: + net_if_unlock(iface); + + return result; } bool z_impl_net_if_ipv6_addr_add_by_index(int index, @@ -4325,27 +4332,34 @@ struct net_if_addr *net_if_ipv4_addr_add(struct net_if *iface, bool net_if_ipv4_addr_rm(struct net_if *iface, const struct in_addr *addr) { struct net_if_ipv4 *ipv4; + bool result = true; int ret; NET_ASSERT(addr); + net_if_lock(iface); + ipv4 = iface->config.ip.ipv4; if (!ipv4) { - return false; + result = false; + goto out; } ret = net_if_addr_unref(iface, AF_INET, addr); if (ret > 0) { NET_DBG("Address %s still in use (ref %d)", net_sprint_ipv4_addr(addr), ret); - return false; - + result = false; + goto out; } else if (ret < 0) { NET_DBG("Address %s not found (%d)", net_sprint_ipv4_addr(addr), ret); } - return true; +out: + net_if_unlock(iface); + + return result; } bool z_impl_net_if_ipv4_addr_add_by_index(int index, From 4841e6f7600586da50b5d38cae956579de0831a5 Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Mon, 10 Feb 2025 10:27:03 +0200 Subject: [PATCH 2/2] net: Update IP address refcount properly when address already exists If an IP address already exists when it is tried to be added to the network interface, then just return it but update ref count if it was not updated. This could happen if the address was added and then removed, but for example an active connection was still using it and keeping the ref count > 0. In this case we must update the ref count so that the IP address is not removed if the connection is closed. Fixes #85380 Signed-off-by: Jukka Rissanen --- include/zephyr/net/net_if.h | 13 ++++++++---- subsys/net/ip/net_if.c | 40 ++++++++++++++++++++++++++++++++++--- subsys/net/ip/tcp.c | 3 ++- 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/include/zephyr/net/net_if.h b/include/zephyr/net/net_if.h index 67501f43448d..392104ca0367 100644 --- a/include/zephyr/net/net_if.h +++ b/include/zephyr/net/net_if.h @@ -132,7 +132,10 @@ struct net_if_addr { */ uint8_t is_temporary : 1; - uint8_t _unused : 4; + /** Was this address added or not */ + uint8_t is_added : 1; + + uint8_t _unused : 3; }; /** @@ -1180,9 +1183,10 @@ int net_if_set_link_addr_locked(struct net_if *iface, extern int net_if_addr_unref_debug(struct net_if *iface, sa_family_t family, const void *addr, + struct net_if_addr **ifaddr, const char *caller, int line); -#define net_if_addr_unref(iface, family, addr) \ - net_if_addr_unref_debug(iface, family, addr, __func__, __LINE__) +#define net_if_addr_unref(iface, family, addr, ifaddr) \ + net_if_addr_unref_debug(iface, family, addr, ifaddr, __func__, __LINE__) extern struct net_if_addr *net_if_addr_ref_debug(struct net_if *iface, sa_family_t family, @@ -1194,7 +1198,8 @@ extern struct net_if_addr *net_if_addr_ref_debug(struct net_if *iface, #else extern int net_if_addr_unref(struct net_if *iface, sa_family_t family, - const void *addr); + const void *addr, + struct net_if_addr **ifaddr); extern struct net_if_addr *net_if_addr_ref(struct net_if *iface, sa_family_t family, const void *addr); diff --git a/subsys/net/ip/net_if.c b/subsys/net/ip/net_if.c index 8e433ddcba56..ef0015f547ac 100644 --- a/subsys/net/ip/net_if.c +++ b/subsys/net/ip/net_if.c @@ -1868,6 +1868,7 @@ static inline void net_if_addr_init(struct net_if_addr *ifaddr, uint32_t vlifetime) { ifaddr->is_used = true; + ifaddr->is_added = true; ifaddr->is_temporary = false; ifaddr->address.family = AF_INET6; ifaddr->addr_type = addr_type; @@ -1906,6 +1907,17 @@ struct net_if_addr *net_if_ipv6_addr_add(struct net_if *iface, ifaddr = ipv6_addr_find(iface, addr); if (ifaddr) { + /* Address already exists, just return it but update ref count + * if it was not updated. This could happen if the address was + * added and then removed but for example an active connection + * was still using it. In this case we must update the ref count + * so that the address is not removed if the connection is closed. + */ + if (!ifaddr->is_added) { + atomic_inc(&ifaddr->atomic_ref); + ifaddr->is_added = true; + } + goto out; } @@ -1963,6 +1975,7 @@ struct net_if_addr *net_if_ipv6_addr_add(struct net_if *iface, bool net_if_ipv6_addr_rm(struct net_if *iface, const struct in6_addr *addr) { + struct net_if_addr *ifaddr; struct net_if_ipv6 *ipv6; bool result = true; int ret; @@ -1977,11 +1990,12 @@ bool net_if_ipv6_addr_rm(struct net_if *iface, const struct in6_addr *addr) goto out; } - ret = net_if_addr_unref(iface, AF_INET6, addr); + ret = net_if_addr_unref(iface, AF_INET6, addr, &ifaddr); if (ret > 0) { NET_DBG("Address %s still in use (ref %d)", net_sprint_ipv6_addr(addr), ret); result = false; + ifaddr->is_added = false; goto out; } else if (ret < 0) { NET_DBG("Address %s not found (%d)", @@ -4265,6 +4279,17 @@ struct net_if_addr *net_if_ipv4_addr_add(struct net_if *iface, ifaddr = ipv4_addr_find(iface, addr); if (ifaddr) { /* TODO: should set addr_type/vlifetime */ + /* Address already exists, just return it but update ref count + * if it was not updated. This could happen if the address was + * added and then removed but for example an active connection + * was still using it. In this case we must update the ref count + * so that the address is not removed if the connection is closed. + */ + if (!ifaddr->is_added) { + atomic_inc(&ifaddr->atomic_ref); + ifaddr->is_added = true; + } + goto out; } @@ -4287,6 +4312,7 @@ struct net_if_addr *net_if_ipv4_addr_add(struct net_if *iface, if (ifaddr) { ifaddr->is_used = true; + ifaddr->is_added = true; ifaddr->address.family = AF_INET; ifaddr->address.in_addr.s4_addr32[0] = addr->s4_addr32[0]; @@ -4331,6 +4357,7 @@ struct net_if_addr *net_if_ipv4_addr_add(struct net_if *iface, bool net_if_ipv4_addr_rm(struct net_if *iface, const struct in_addr *addr) { + struct net_if_addr *ifaddr; struct net_if_ipv4 *ipv4; bool result = true; int ret; @@ -4345,11 +4372,12 @@ bool net_if_ipv4_addr_rm(struct net_if *iface, const struct in_addr *addr) goto out; } - ret = net_if_addr_unref(iface, AF_INET, addr); + ret = net_if_addr_unref(iface, AF_INET, addr, &ifaddr); if (ret > 0) { NET_DBG("Address %s still in use (ref %d)", net_sprint_ipv4_addr(addr), ret); result = false; + ifaddr->is_added = false; goto out; } else if (ret < 0) { NET_DBG("Address %s not found (%d)", @@ -4973,11 +5001,13 @@ struct net_if_addr *net_if_addr_ref(struct net_if *iface, int net_if_addr_unref_debug(struct net_if *iface, sa_family_t family, const void *addr, + struct net_if_addr **ret_ifaddr, const char *caller, int line) #else int net_if_addr_unref(struct net_if *iface, sa_family_t family, - const void *addr) + const void *addr, + struct net_if_addr **ret_ifaddr) #endif /* NET_LOG_LEVEL >= LOG_LEVEL_DBG */ { struct net_if_addr *ifaddr; @@ -5031,6 +5061,10 @@ int net_if_addr_unref(struct net_if *iface, #endif if (ref > 1) { + if (ret_ifaddr) { + *ret_ifaddr = ifaddr; + } + return ref - 1; } diff --git a/subsys/net/ip/tcp.c b/subsys/net/ip/tcp.c index dd8f1f5108e8..eb65bf38e78f 100644 --- a/subsys/net/ip/tcp.c +++ b/subsys/net/ip/tcp.c @@ -783,7 +783,8 @@ static void tcp_conn_release(struct k_work *work) net_if_addr_unref(conn->iface, conn->src.sa.sa_family, conn->src.sa.sa_family == AF_INET ? (const void *)&conn->src.sin.sin_addr : - (const void *)&conn->src.sin6.sin6_addr); + (const void *)&conn->src.sin6.sin6_addr, + NULL); } conn->context->tcp = NULL;