From b5b441abaa852e85ddefd8b22835c9b85898cc06 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Fabian=20Gr=C3=BCnbichler?= Date: Wed, 22 Mar 2017 16:13:50 +0100 Subject: [PATCH] common: fix segfault in public IPv6 addr picking MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 (cherry picked from commit ae2ee3d3835fe25b35eeb1a841ee5234cd69eb65) --- src/common/ipaddr.cc | 6 ++-- src/common/pick_address.cc | 4 +-- src/include/ipaddr.h | 2 +- src/test/test_ipaddr.cc | 60 ++++++++++++++++++++++++-------------- 4 files changed, 44 insertions(+), 28 deletions(-) diff --git a/src/common/ipaddr.cc b/src/common/ipaddr.cc index 72fe7de2c894c..a7dedf257095b 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 a47a1d946a4cc..648ec21363631 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 cac13d62c3e03..bf1a08308991d 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 240a3a75b56da..a7e8ed38c1c1c 100644 --- a/src/test/test_ipaddr.cc +++ b/src/test/test_ipaddr.cc @@ -252,7 +252,7 @@ TEST(CommonIPAddr, TestV6_PrefixZero) TEST(CommonIPAddr, ParseNetwork_Empty) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -262,7 +262,7 @@ TEST(CommonIPAddr, ParseNetwork_Empty) TEST(CommonIPAddr, ParseNetwork_Bad_Junk) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -272,27 +272,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; @@ -302,7 +302,7 @@ TEST(CommonIPAddr, ParseNetwork_Bad_IPv4) TEST(CommonIPAddr, ParseNetwork_Bad_IPv4Slash) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -312,7 +312,7 @@ TEST(CommonIPAddr, ParseNetwork_Bad_IPv4Slash) TEST(CommonIPAddr, ParseNetwork_Bad_IPv4SlashNegative) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -322,7 +322,7 @@ TEST(CommonIPAddr, ParseNetwork_Bad_IPv4SlashNegative) TEST(CommonIPAddr, ParseNetwork_Bad_IPv4SlashJunk) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -332,7 +332,7 @@ TEST(CommonIPAddr, ParseNetwork_Bad_IPv4SlashJunk) TEST(CommonIPAddr, ParseNetwork_Bad_IPv6) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -342,7 +342,7 @@ TEST(CommonIPAddr, ParseNetwork_Bad_IPv6) TEST(CommonIPAddr, ParseNetwork_Bad_IPv6Slash) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -352,7 +352,7 @@ TEST(CommonIPAddr, ParseNetwork_Bad_IPv6Slash) TEST(CommonIPAddr, ParseNetwork_Bad_IPv6SlashNegative) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -362,7 +362,7 @@ TEST(CommonIPAddr, ParseNetwork_Bad_IPv6SlashNegative) TEST(CommonIPAddr, ParseNetwork_Bad_IPv6SlashJunk) { - struct sockaddr network; + struct sockaddr_storage network; unsigned int prefix_len; bool ok; @@ -373,10 +373,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); @@ -389,10 +391,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); @@ -405,10 +409,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); @@ -421,10 +427,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); @@ -437,10 +445,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); @@ -453,10 +463,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); @@ -469,10 +481,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); @@ -485,10 +499,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); -- 2.39.5