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 <f.gruenbichler@proxmox.com>

diff --git a/src/common/ipaddr.cc b/src/common/ipaddr.cc
index 72fe7de2c894c94b77b0dd9f836ebeb8db540b0c..a7dedf257095beb28287644794d92eeb3b3c68f2 100644 (file)
--- 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 06d58e46cd6df3284fea6b1d06ccf0c6c1079fb2..5f910d541e63d172a91511979f09697901866eab 100644 (file)
--- 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<string>::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 cac13d62c3e035d43e67bcfbcfa769de8c6182a2..bf1a08308991d46e2ee9f645bcb6deb13068b4e8 100644 (file)
--- 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 9a3d43ea6d430e21312d702f3afd0563e07a23b5..716094d80d85e5f538346788cbb648590f34f445 100644 (file)
--- 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);