From: Fabian Grünbichler Date: Wed, 22 Mar 2017 15:13:50 +0000 (+0100) Subject: common: fix segfault in public IPv6 addr picking X-Git-Tag: v12.0.2~234^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=ae2ee3d3835fe25b35eeb1a841ee5234cd69eb65;p=ceph-ci.git common: fix segfault in public IPv6 addr picking sockaddr is only 16 bytes big, so declaring net as sockaddr and then casting to sockaddr_in6 in case of IPv6 cannot work. using sockaddr_storage works for both IPv4 and IPv6, and is used in other code parts as well. note that the tests did not find this issue as they declared the bigger structs and casted the references to (sockaddr *) Fixes: http://tracker.ceph.com/issues/19371 Signed-off-by: Fabian Grünbichler --- diff --git a/src/common/ipaddr.cc b/src/common/ipaddr.cc index 72fe7de2c89..a7dedf25709 100644 --- a/src/common/ipaddr.cc +++ b/src/common/ipaddr.cc @@ -110,7 +110,7 @@ const struct sockaddr *find_ip_in_subnet(const struct ifaddrs *addrs, } -bool parse_network(const char *s, struct sockaddr *network, unsigned int *prefix_len) { +bool parse_network(const char *s, struct sockaddr_storage *network, unsigned int *prefix_len) { char *slash = strchr((char*)s, '/'); if (!slash) { // no slash @@ -144,14 +144,14 @@ bool parse_network(const char *s, struct sockaddr *network, unsigned int *prefix int ok; ok = inet_pton(AF_INET, addr, &((struct sockaddr_in*)network)->sin_addr); if (ok) { - network->sa_family = AF_INET; + network->ss_family = AF_INET; return true; } // try parsing as ipv6 ok = inet_pton(AF_INET6, addr, &((struct sockaddr_in6*)network)->sin6_addr); if (ok) { - network->sa_family = AF_INET6; + network->ss_family = AF_INET6; return true; } diff --git a/src/common/pick_address.cc b/src/common/pick_address.cc index 06d58e46cd6..5f910d541e6 100644 --- a/src/common/pick_address.cc +++ b/src/common/pick_address.cc @@ -32,7 +32,7 @@ static const struct sockaddr *find_ip_in_subnet_list(CephContext *cct, get_str_list(networks, nets); for(std::list::iterator s = nets.begin(); s != nets.end(); ++s) { - struct sockaddr net; + struct sockaddr_storage net; unsigned int prefix_len; if (!parse_network(s->c_str(), &net, &prefix_len)) { @@ -40,7 +40,7 @@ static const struct sockaddr *find_ip_in_subnet_list(CephContext *cct, exit(1); } - const struct sockaddr *found = find_ip_in_subnet(ifa, &net, prefix_len); + const struct sockaddr *found = find_ip_in_subnet(ifa, (struct sockaddr *) &net, prefix_len); if (found) return found; } diff --git a/src/include/ipaddr.h b/src/include/ipaddr.h index cac13d62c3e..bf1a0830899 100644 --- a/src/include/ipaddr.h +++ b/src/include/ipaddr.h @@ -16,6 +16,6 @@ const struct sockaddr *find_ip_in_subnet(const struct ifaddrs *addrs, unsigned int prefix_len); -bool parse_network(const char *s, struct sockaddr *network, unsigned int *prefix_len); +bool parse_network(const char *s, struct sockaddr_storage *network, unsigned int *prefix_len); #endif diff --git a/src/test/test_ipaddr.cc b/src/test/test_ipaddr.cc index 9a3d43ea6d4..716094d80d8 100644 --- a/src/test/test_ipaddr.cc +++ b/src/test/test_ipaddr.cc @@ -256,7 +256,7 @@ TEST(CommonIPAddr, TestV6_PrefixZero) TEST(CommonIPAddr, ParseNetwork_Empty) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -266,7 +266,7 @@ TEST(CommonIPAddr, ParseNetwork_Empty) TEST(CommonIPAddr, ParseNetwork_Bad_Junk) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -276,27 +276,27 @@ TEST(CommonIPAddr, ParseNetwork_Bad_Junk) TEST(CommonIPAddr, ParseNetwork_Bad_SlashNum) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; - ok = parse_network("/24", (struct sockaddr*)&network, &prefix_len); + ok = parse_network("/24", &network, &prefix_len); ASSERT_EQ(ok, false); } TEST(CommonIPAddr, ParseNetwork_Bad_Slash) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; - ok = parse_network("/", (struct sockaddr*)&network, &prefix_len); + ok = parse_network("/", &network, &prefix_len); ASSERT_EQ(ok, false); } TEST(CommonIPAddr, ParseNetwork_Bad_IPv4) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -306,7 +306,7 @@ TEST(CommonIPAddr, ParseNetwork_Bad_IPv4) TEST(CommonIPAddr, ParseNetwork_Bad_IPv4Slash) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -316,7 +316,7 @@ TEST(CommonIPAddr, ParseNetwork_Bad_IPv4Slash) TEST(CommonIPAddr, ParseNetwork_Bad_IPv4SlashNegative) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -326,7 +326,7 @@ TEST(CommonIPAddr, ParseNetwork_Bad_IPv4SlashNegative) TEST(CommonIPAddr, ParseNetwork_Bad_IPv4SlashJunk) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -336,7 +336,7 @@ TEST(CommonIPAddr, ParseNetwork_Bad_IPv4SlashJunk) TEST(CommonIPAddr, ParseNetwork_Bad_IPv6) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -346,7 +346,7 @@ TEST(CommonIPAddr, ParseNetwork_Bad_IPv6) TEST(CommonIPAddr, ParseNetwork_Bad_IPv6Slash) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -356,7 +356,7 @@ TEST(CommonIPAddr, ParseNetwork_Bad_IPv6Slash) TEST(CommonIPAddr, ParseNetwork_Bad_IPv6SlashNegative) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -366,7 +366,7 @@ TEST(CommonIPAddr, ParseNetwork_Bad_IPv6SlashNegative) TEST(CommonIPAddr, ParseNetwork_Bad_IPv6SlashJunk) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -377,10 +377,12 @@ TEST(CommonIPAddr, ParseNetwork_Bad_IPv6SlashJunk) TEST(CommonIPAddr, ParseNetwork_IPv4_0) { struct sockaddr_in network; + struct sockaddr_storage net_storage; unsigned int prefix_len; bool ok; - ok = parse_network("123.123.123.123/0", (struct sockaddr*)&network, &prefix_len); + ok = parse_network("123.123.123.123/0", &net_storage, &prefix_len); + network = *(struct sockaddr_in *) &net_storage; ASSERT_EQ(ok, true); ASSERT_EQ(0U, prefix_len); ASSERT_EQ(AF_INET, network.sin_family); @@ -393,10 +395,12 @@ TEST(CommonIPAddr, ParseNetwork_IPv4_0) TEST(CommonIPAddr, ParseNetwork_IPv4_13) { struct sockaddr_in network; + struct sockaddr_storage net_storage; unsigned int prefix_len; bool ok; - ok = parse_network("123.123.123.123/13", (struct sockaddr*)&network, &prefix_len); + ok = parse_network("123.123.123.123/13", &net_storage, &prefix_len); + network = *(struct sockaddr_in *) &net_storage; ASSERT_EQ(ok, true); ASSERT_EQ(13U, prefix_len); ASSERT_EQ(AF_INET, network.sin_family); @@ -409,10 +413,12 @@ TEST(CommonIPAddr, ParseNetwork_IPv4_13) TEST(CommonIPAddr, ParseNetwork_IPv4_32) { struct sockaddr_in network; + struct sockaddr_storage net_storage; unsigned int prefix_len; bool ok; - ok = parse_network("123.123.123.123/32", (struct sockaddr*)&network, &prefix_len); + ok = parse_network("123.123.123.123/32", &net_storage, &prefix_len); + network = *(struct sockaddr_in *) &net_storage; ASSERT_EQ(ok, true); ASSERT_EQ(32U, prefix_len); ASSERT_EQ(AF_INET, network.sin_family); @@ -425,10 +431,12 @@ TEST(CommonIPAddr, ParseNetwork_IPv4_32) TEST(CommonIPAddr, ParseNetwork_IPv4_42) { struct sockaddr_in network; + struct sockaddr_storage net_storage; unsigned int prefix_len; bool ok; - ok = parse_network("123.123.123.123/42", (struct sockaddr*)&network, &prefix_len); + ok = parse_network("123.123.123.123/42", &net_storage, &prefix_len); + network = *(struct sockaddr_in *) &net_storage; ASSERT_EQ(ok, true); ASSERT_EQ(42U, prefix_len); ASSERT_EQ(AF_INET, network.sin_family); @@ -441,10 +449,12 @@ TEST(CommonIPAddr, ParseNetwork_IPv4_42) TEST(CommonIPAddr, ParseNetwork_IPv6_0) { struct sockaddr_in6 network; + struct sockaddr_storage net_storage; unsigned int prefix_len; bool ok; - ok = parse_network("2001:1234:5678:90ab::dead:beef/0", (struct sockaddr*)&network, &prefix_len); + ok = parse_network("2001:1234:5678:90ab::dead:beef/0", &net_storage, &prefix_len); + network = *(struct sockaddr_in6 *) &net_storage; ASSERT_EQ(ok, true); ASSERT_EQ(0U, prefix_len); ASSERT_EQ(AF_INET6, network.sin6_family); @@ -457,10 +467,12 @@ TEST(CommonIPAddr, ParseNetwork_IPv6_0) TEST(CommonIPAddr, ParseNetwork_IPv6_67) { struct sockaddr_in6 network; + struct sockaddr_storage net_storage; unsigned int prefix_len; bool ok; - ok = parse_network("2001:1234:5678:90ab::dead:beef/67", (struct sockaddr*)&network, &prefix_len); + ok = parse_network("2001:1234:5678:90ab::dead:beef/67", &net_storage, &prefix_len); + network = *(struct sockaddr_in6 *) &net_storage; ASSERT_EQ(ok, true); ASSERT_EQ(67U, prefix_len); ASSERT_EQ(AF_INET6, network.sin6_family); @@ -473,10 +485,12 @@ TEST(CommonIPAddr, ParseNetwork_IPv6_67) TEST(CommonIPAddr, ParseNetwork_IPv6_128) { struct sockaddr_in6 network; + struct sockaddr_storage net_storage; unsigned int prefix_len; bool ok; - ok = parse_network("2001:1234:5678:90ab::dead:beef/128", (struct sockaddr*)&network, &prefix_len); + ok = parse_network("2001:1234:5678:90ab::dead:beef/128", &net_storage, &prefix_len); + network = *(struct sockaddr_in6 *) &net_storage; ASSERT_EQ(ok, true); ASSERT_EQ(128U, prefix_len); ASSERT_EQ(AF_INET6, network.sin6_family); @@ -489,10 +503,12 @@ TEST(CommonIPAddr, ParseNetwork_IPv6_128) TEST(CommonIPAddr, ParseNetwork_IPv6_9000) { struct sockaddr_in6 network; + struct sockaddr_storage net_storage; unsigned int prefix_len; bool ok; - ok = parse_network("2001:1234:5678:90ab::dead:beef/9000", (struct sockaddr*)&network, &prefix_len); + ok = parse_network("2001:1234:5678:90ab::dead:beef/9000", &net_storage, &prefix_len); + network = *(struct sockaddr_in6 *) &net_storage; ASSERT_EQ(ok, true); ASSERT_EQ(9000U, prefix_len); ASSERT_EQ(AF_INET6, network.sin6_family);