Skip to content

Commit

Permalink
common: fix segfault in public IPv6 addr picking
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@Fabian-Gruenbichler
Fabian-Gruenbichler committed Mar 28, 2017
1 parent c52431b commit ae2ee3d
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 28 deletions.
6 changes: 3 additions & 3 deletions src/common/ipaddr.cc
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions src/common/pick_address.cc
View file
Expand Up @@ -32,15 +32,15 @@ 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)) {
lderr(cct) << "unable to parse network: " << *s << dendl;
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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/include/ipaddr.h
View file
Expand Up @@ -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
60 changes: 38 additions & 22 deletions src/test/test_ipaddr.cc
View file
Expand Up @@ -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;

Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit ae2ee3d

Please sign in to comment.