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>
(cherry picked from commit ae2ee3d3835fe25b35eeb1a841ee5234cd69eb65)

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 a47a1d946a4cc276b5ee78389d2ef4d831dc344b..648ec213636313879fd4f8f68956f2165e64a26c 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 240a3a75b56dac3cedb3347336952ee8ebe82f7c..a7e8ed38c1c1cf6c350d48caefc344540707bba2 100644 (file)
--- 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);