Andrei G has uploaded this change for review.
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));
To view, visit change 43575. To unsubscribe, or for help writing mail filters, visit settings.