Andrei G has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/43575?usp=email )
Change subject: core/socket: sockaddr_cmp: compare fields, not bytes ......................................................................
core/socket: sockaddr_cmp: compare fields, not bytes
osmo_sockaddr_cmp() decides whether two addresses are equal with a memcmp() over the whole struct sockaddr_in or sockaddr_in6. On Linux those structures hold only family, port, address and padding, so the comparison is exact. On Darwin and the BSDs the first byte is sin_len / sin6_len. The kernel fills it in on recvfrom() and accept(), while an address the application built from configuration leaves it zero. The same peer then compares as two different addresses.
Every static NS-VC in gprs_ns2 breaks on this. The answer to the first NS-RESET arrives from the configured remote, gprs_ns2_udp.c cannot match it to the NS-VC ("Ignoring NS RESET ACK from newconnection for non-existing NS-VC", gprs_ns2.c:1066) and the link never leaves RESET. Observed with osmo-pcu against osmo-sgsn on Darwin loopback: the SGSN side, which learned the peer from the packet, goes to BLOCKED and then loses every NS-ALIVE-ACK, while the PCU side stays in RESET. Dynamic NS-VCs are unaffected because their remote address is a copy of what recvfrom() returned.
Compare the fields instead of the bytes: port and address for AF_INET, port, flow info, address and scope id for AF_INET6, in the order memcmp() visited them, so the ordering the function gives to sorted users does not change. The default branch keeps its memcmp() over the full osmo_sockaddr. Linux behaviour is unchanged.
Change-Id: I75fc62a92f546a906dbb1440984d62f962579255 Signed-off-by: Andrei Gosman andrei.gosman@gmail.com --- M src/core/socket.c 1 file changed, 30 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/75/43575/1
diff --git a/src/core/socket.c b/src/core/socket.c index 3a2d7b7..724d8ef 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2599,11 +2599,37 @@ return OSMO_CMP(a->u.sa.sa_family, b->u.sa.sa_family); }
+ /* Compare the address fields, not the raw bytes of the structure. + * On the BSDs and Darwin the sockaddr carries a length byte (sin_len, + * sin6_len) that the kernel fills in on recvfrom()/accept() and that + * is zero in an address the application built itself; a memcmp() over + * the whole structure then reports two equal addresses as different. + * The order of the comparisons keeps the ordering memcmp() gave. */ switch (a->u.sa.sa_family) { - case AF_INET: - return memcmp(&a->u.sin, &b->u.sin, sizeof(struct sockaddr_in)); - case AF_INET6: - return memcmp(&a->u.sin6, &b->u.sin6, sizeof(struct sockaddr_in6)); + case AF_INET: { + int rc = memcmp(&a->u.sin.sin_port, &b->u.sin.sin_port, + sizeof(a->u.sin.sin_port)); + if (rc) + return rc; + return memcmp(&a->u.sin.sin_addr, &b->u.sin.sin_addr, + sizeof(a->u.sin.sin_addr)); + } + case AF_INET6: { + int rc = memcmp(&a->u.sin6.sin6_port, &b->u.sin6.sin6_port, + sizeof(a->u.sin6.sin6_port)); + if (rc) + return rc; + rc = memcmp(&a->u.sin6.sin6_flowinfo, &b->u.sin6.sin6_flowinfo, + sizeof(a->u.sin6.sin6_flowinfo)); + if (rc) + return rc; + rc = memcmp(&a->u.sin6.sin6_addr, &b->u.sin6.sin6_addr, + sizeof(a->u.sin6.sin6_addr)); + if (rc) + return rc; + return memcmp(&a->u.sin6.sin6_scope_id, &b->u.sin6.sin6_scope_id, + sizeof(a->u.sin6.sin6_scope_id)); + } default: /* fallback to memcmp for remaining AF over the full osmo_sockaddr length */ return memcmp(a, b, sizeof(struct osmo_sockaddr));