On 10/07/2013 03:32 PM, Jakub Hrozek wrote:
On Fri, Oct 04, 2013 at 12:52:31PM +0200, Michal Židek wrote:
On 10/04/2013 12:37 PM, Michal Židek wrote:
On 10/03/2013 08:42 PM, Jakub Hrozek wrote:
On Tue, Oct 01, 2013 at 07:04:19PM +0200, Michal Židek wrote:
On 10/01/2013 06:59 PM, Michal Židek wrote:
Hello,
these are the patches, that solve the casting of sockaddr structure.
Patch 0001 - Adds function to filter out special ip addresses
(loopback,
multicast, linklocal, broadcast) and also a unit test for it. Used in
later patches.
Patch 0002 and 0003 - Use the function from patch 0001 and properly
copy
the sockaddr_in structure to avoid alignment warnings.
Patch 0004 - Avoid casting of sockaddr to sockaddr_in in tests.
Also here is the discussion from diffrent thread, related to sockaddr
casting:
// PASTE BEGIN
I tested some compilers and patches work fine with clang and gcc.
But there are also another compilers in the world
ant they needn't understand "pragma GCC". This pragma is not
standardized.
src/providers/dp_dyndns.c(145): warning #161: unrecognized #pragma
#pragma GCC diagnostic push
^
src/providers/dp_dyndns.c(146): warning #161: unrecognized #pragma
#pragma GCC diagnostic ignored "-Wcast-align"
^
src/providers/dp_dyndns.c(206): warning #161: unrecognized #pragma
#pragma GCC diagnostic pop
^
If you are 100% sure about alignment, you can define own macro.
Something like this:
#define ignore_align(var, dest_type) \
(dest_type *)((void*)(var))
And usage:
wrapper->refcount = ignore_align(wrapper->ptr +
refcount_offset, int);
Does anybody have any better ideas?
Compiler specific #pragma should not be used. And I think in
general
#pragma should be used with great care. Currently there is only
one use
case in sssd in the memcache code to align some structs which make
sense.
Indeed, agree 100%
Especially I think ingnoring a warning is not a good idea. If
currently
no one has a good idea how to handle the sockaddr and in_addr
structs in
a way which does not produce a warning I would prefer to keep the
warning around until someone has a good idea. Imo a comment in the
code
that the warning can be ignored would be sufficient for the time
being.
casting sockaddr is a bug, unless you know for sure it was
created as
sockaddr_un and then simply casted to sockaddr, but in that case I
would
consider the casting to pass it around just a different bug.
Please use sockaddr_un anywhere you need to perform casts. Pass
around
sockaddr_un and cast it only at the time of use when needed.
It is not possible to use sockaddr_un(or sockaddr_storage).
We use function getifaddrs to retrieve addresses of network
interfaces.
Manual page says (man 3 getifaddrs)
--------------------------------------------------------------------------
int getifaddrs(struct ifaddrs **ifap);
The getifaddrs() function creates a linked list of
structures
describing the network interfaces of the local system, and
stores
the address of the first item of the list in *ifap.
The list consists of ifaddrs structures, defined as follows:
struct ifaddrs {
struct ifaddrs *ifa_next; /* Next item in
list */
char *ifa_name; /* Name of
interface */
unsigned int ifa_flags; /* Flags from
SIOCGIFFLAGS */
struct sockaddr *ifa_addr; /* Address of
interface */
^^^^^^^^^^^^^^^
We cannot change this type
struct sockaddr *ifa_netmask; /* Netmask of
interface */
union {
struct sockaddr *ifu_broadaddr;
/* Broadcast address of
interface */
struct sockaddr *ifu_dstaddr;
/* Point-to-point destination
address */
} ifa_ifu;
#define ifa_broadaddr ifa_ifu.ifu_broadaddr
#define ifa_dstaddr ifa_ifu.ifu_dstaddr
void *ifa_data; /* Address-specific
data */
};
--------------------------------------------------------------------------
In this case you should copy the contents of sockaddr to a new
variable
of the right type, and not cast.
Simo.
Yes, this is the only way you can deal with these issues.
I remembered this from my experiments with ELAPI 3 years ago.
I ran into similar problems with this function and had to copy data.
And now with patches.
Michal
+/* addr is in network order */
+bool filter_ipv4_addr(struct in_addr *addr, uint8_t filter)
+{
+ char straddr[INET_ADDRSTRLEN];
+
+ if (inet_ntop(AF_INET, addr, straddr, INET_ADDRSTRLEN) == NULL) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ ("inet_ntop failed, won't log IP addresses\n"));
+ snprintf(straddr, INET_ADDRSTRLEN, "unknown");
+ }
+
+ if ((filter & SSS_NO_MULTICAST) &&
IN_MULTICAST(ntohl(addr->s_addr))) {
+ DEBUG(SSSDBG_FUNC_DATA, ("Multicast IPv4 address %s\n",
straddr));
+ return false;
+ } else if ((filter & SSS_NO_LOOPBACK)
+ && inet_netof(*addr) == IN_LOOPBACKNET) {
+ DEBUG(SSSDBG_FUNC_DATA, ("Loopback IPv4 address %s\n",
straddr));
+ return false;
+ } else if ((filter & SSS_NO_LINKLOCAL)
+ && (addr->s_addr & htonl(0xffff0000)) ==
htonl(0xa9fe0000)) {
+ /* 169.254.0.0/16 */
+ DEBUG(SSSDBG_FUNC_DATA, ("Link-local IPv4 address %s\n",
straddr));
+ return false;
+ } else if ((filter & SSS_NO_BROADCAST)
+ && addr->s_addr == htonl(INADDR_BROADCAST)) {
+ DEBUG(SSSDBG_FUNC_DATA, ("Broadcast IPv4 address %s\n",
straddr));
+ return false;
+ }
+
+ return true;
+}
In general the patches look good to me and seem to work well, but I
don't like the fact that the function is called "filter_" but returns
true if nothing is filtered. I think we should either revert the naming
or the return booleans.
Thank you for the review.
What about check_ipv4/6_addr ? If addr satisfies all given flags (I
renamed the last parameter to flags) it returns true, otherwise false.
New patches attached.
Michal
I accidentally removed one line in util.h.
New patches attached.
OK, I like the new naming. Two nitpicks:
From 936c2bde8b36877ddba71a7dceb3e3ee861e50f5 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzi...@redhat.com>
Date: Tue, 1 Oct 2013 16:36:55 +0200
Subject: [PATCH 2/4] dyndns: Use filter_ipvX_addr functions
^^^^^^^^^^^^^
The commit message should use the new function
names
Sure, sorry for that.
---
src/providers/dp_dyndns.c | 51 ++++++-----------------------------------------
1 file changed, 6 insertions(+), 45 deletions(-)
diff --git a/src/providers/dp_dyndns.c b/src/providers/dp_dyndns.c
index cd11431..77a128f 100644
--- a/src/providers/dp_dyndns.c
+++ b/src/providers/dp_dyndns.c
@@ -144,55 +144,16 @@ fail:
static bool
ok_for_dns(struct sockaddr *sa)
{
- char straddr[INET6_ADDRSTRLEN];
- struct in6_addr *addr6;
- struct in_addr *addr;
+ struct sockaddr_in sa4;
+ struct sockaddr_in6 sa6;
switch (sa->sa_family) {
case AF_INET6:
- addr6 = &((struct sockaddr_in6 *) sa)->sin6_addr;
-
- if (inet_ntop(AF_INET6, addr6, straddr, INET6_ADDRSTRLEN) == NULL) {
- DEBUG(SSSDBG_MINOR_FAILURE,
- ("inet_ntop failed, won't log IP addresses\n"));
- snprintf(straddr, INET6_ADDRSTRLEN, "unknown");
- }
-
- if (IN6_IS_ADDR_LINKLOCAL(addr6)) {
- DEBUG(SSSDBG_FUNC_DATA, ("Link local IPv6 address %s\n", straddr));
- return false;
- } else if (IN6_IS_ADDR_LOOPBACK(addr6)) {
- DEBUG(SSSDBG_FUNC_DATA, ("Loopback IPv6 address %s\n", straddr));
- return false;
- } else if (IN6_IS_ADDR_MULTICAST(addr6)) {
- DEBUG(SSSDBG_FUNC_DATA, ("Multicast IPv6 address %s\n", straddr));
- return false;
- }
- break;
+ memcpy(&sa6, sa, sizeof(struct sockaddr_in6));
+ return check_ipv6_addr(&sa6.sin6_addr, SSS_NO_SPECIAL);
I wonder if the function could simply accept sockaddr_storage or
sockaddr and do the conversion inside?
It was intentional to have one function for IPv4 and one for IPv6. We
already know what version of IP are we dealing with in the upper layer,
that layer is also the place where the safealign issue is solved. If we
pass only sockaddr to this function we would have to deal with the same
problem as in the upper layer (from the perspective of the function, we
do not know that the sockaddr we got is already safe to cast, and that
it was properly copied in upper layer, so we would need to copy it
again). Even if sockaddr_storage was used, we would still need to do the
memcpy in the upper layer in separate branches for IPv4 and IPv6, so
there is no reason to "forget" this information when calling another
function.
If there will be a place in the future, where it would be beneficial to
have one function that deals with both IP versions, we can make a
wrapper on top of these two functions. It would be simple to do, but so
far, there is no such place in the code, so I did not make such wrapper
in this patchset.
case AF_INET:
- addr = &((struct sockaddr_in *) sa)->sin_addr;
-
- if (inet_ntop(AF_INET, addr, straddr, INET6_ADDRSTRLEN) == NULL) {
- DEBUG(SSSDBG_MINOR_FAILURE,
- ("inet_ntop failed, won't log IP addresses\n"));
- snprintf(straddr, INET6_ADDRSTRLEN, "unknown");
- }
-
- if (IN_MULTICAST(ntohl(addr->s_addr))) {
- DEBUG(SSSDBG_FUNC_DATA, ("Multicast IPv4 address %s\n", straddr));
- return false;
- } else if (inet_netof(*addr) == IN_LOOPBACKNET) {
- DEBUG(SSSDBG_FUNC_DATA, ("Loopback IPv4 address %s\n", straddr));
- return false;
- } else if ((addr->s_addr & htonl(0xffff0000)) == htonl(0xa9fe0000)) {
- /* 169.254.0.0/16 */
- DEBUG(SSSDBG_FUNC_DATA, ("Link-local IPv4 address %s\n", straddr));
- return false;
- } else if (addr->s_addr == htonl(INADDR_BROADCAST)) {
- DEBUG(SSSDBG_FUNC_DATA, ("Broadcast IPv4 address %s\n", straddr));
- return false;
- }
- break;
+ memcpy(&sa4, sa, sizeof(struct sockaddr_in));
+ return check_ipv4_addr(&sa4.sin_addr, SSS_NO_SPECIAL);
default:
DEBUG(SSSDBG_CRIT_FAILURE, ("Unknown address family\n"));
return false;
--
1.7.11.2
And about the unit tests, could we also add the address the reporter had
problems with (192.168.254.169) to make sure we don't regress?
Ok. I added this IP to the tests.
Updated patches in attachment.
Michal
>From 2f57d11daeef9438f6c5d2c7f2f491f11ac2edd4 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzi...@redhat.com>
Date: Tue, 1 Oct 2013 16:22:13 +0200
Subject: [PATCH 1/4] util: Add functions to check if IP addresses is special
Added functions to check if given IP address is a special address
(broadcast, multicast...).
---
src/tests/util-tests.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++
src/util/util.c | 57 +++++++++++++++++++++++
src/util/util.h | 13 ++++++
3 files changed, 189 insertions(+)
diff --git a/src/tests/util-tests.c b/src/tests/util-tests.c
index fd3dfb9..3ca6e6e 100644
--- a/src/tests/util-tests.c
+++ b/src/tests/util-tests.c
@@ -808,6 +808,123 @@ START_TEST(test_split_on_separator)
}
END_TEST
+struct check_ip_test_data {
+ const char *str_ipaddr;
+ uint8_t flags;
+ bool expected_ret;
+};
+
+START_TEST(test_check_ipv4_addr)
+{
+ int a;
+ int num_of_tests;
+ int ret;
+ bool bret;
+ struct in_addr addr;
+ struct check_ip_test_data tst_data[] = {
+ {
+ "192.168.100.1", /* input IPv4 address */
+ 0, /* flags value */
+ true /* Expected return value */
+ },
+ {
+ "224.0.0.22", /* multicast address */
+ SSS_NO_MULTICAST,
+ false
+ },
+ {
+ "127.0.0.1",
+ SSS_NO_LOOPBACK,
+ false
+ },
+ {
+ "169.254.0.11",
+ SSS_NO_LINKLOCAL,
+ false
+ },
+ {
+ "255.255.255.255",
+ SSS_NO_BROADCAST,
+ false
+ },
+ {
+ "255.255.255.255",
+ SSS_NO_SPECIAL,
+ false
+ },
+ {
+ "192.168.254.169",
+ SSS_NO_SPECIAL,
+ true
+ },
+ };
+
+ num_of_tests = sizeof(tst_data) / sizeof(struct check_ip_test_data);
+
+ for (a = 0; a < num_of_tests; a++) {
+ /* fill sockaddr_in structure */
+
+ ret = inet_pton(AF_INET, tst_data[a].str_ipaddr, &addr);
+ fail_if(ret != 1, "inet_pton failed.");
+
+ bret = check_ipv4_addr(&addr, tst_data[a].flags);
+ fail_unless(bret == tst_data[a].expected_ret,
+ "check_ipv4_addr failed (iteration %d)", a);
+ }
+}
+END_TEST
+
+START_TEST(test_check_ipv6_addr)
+{
+ int a;
+ int num_of_tests;
+ int ret;
+ bool bret;
+ struct in6_addr addr;
+ struct check_ip_test_data tst_data[] = {
+ {
+ "fde9:7e3f:1ed3:24a5::4", /* input IPv6 address */
+ 0, /* flags value */
+ true /* Expected return value */
+ },
+ {
+ "fe80::f2de:f1ff:fefa:67f0",
+ SSS_NO_LINKLOCAL,
+ false
+ },
+ {
+ "::1",
+ SSS_NO_LOOPBACK,
+ false
+ },
+ {
+ "ff00::123",
+ SSS_NO_MULTICAST,
+ false
+ },
+ {
+ "ff00::321",
+ SSS_NO_SPECIAL,
+ false
+ },
+ };
+
+ num_of_tests = sizeof(tst_data) / sizeof(struct check_ip_test_data);
+
+ for (a = 0; a < num_of_tests; a++) {
+ /* fill sockaddr_in structure */
+
+ ret = inet_pton(AF_INET6, tst_data[a].str_ipaddr, &addr);
+ fail_if(ret != 1, "inet_pton failed.");
+
+ bret = check_ipv6_addr(&addr, tst_data[a].flags);
+ fail_unless(bret == tst_data[a].expected_ret,
+ "check_ipv6_addr failed (iteration %d)", a);
+
+ }
+}
+END_TEST
+
START_TEST(test_is_host_in_domain)
{
struct {
@@ -851,6 +968,8 @@ Suite *util_suite(void)
tcase_add_test (tc_util, test_add_string_to_list);
tcase_add_test (tc_util, test_string_in_list);
tcase_add_test (tc_util, test_split_on_separator);
+ tcase_add_test (tc_util, test_check_ipv4_addr);
+ tcase_add_test (tc_util, test_check_ipv6_addr);
tcase_add_test (tc_util, test_is_host_in_domain);
tcase_set_timeout(tc_util, 60);
diff --git a/src/util/util.c b/src/util/util.c
index fb3bed1..9ab154d 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -22,6 +22,7 @@
#include <netdb.h>
#include <poll.h>
#include <sys/socket.h>
+#include <arpa/inet.h>
#include "talloc.h"
#include "util/util.h"
@@ -739,3 +740,59 @@ bool is_host_in_domain(const char *host, const char *domain)
return false;
}
+
+/* addr is in network order for both IPv4 and IPv6 versions */
+bool check_ipv4_addr(struct in_addr *addr, uint8_t flags)
+{
+ char straddr[INET_ADDRSTRLEN];
+
+ if (inet_ntop(AF_INET, addr, straddr, INET_ADDRSTRLEN) == NULL) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ ("inet_ntop failed, won't log IP addresses\n"));
+ snprintf(straddr, INET_ADDRSTRLEN, "unknown");
+ }
+
+ if ((flags & SSS_NO_MULTICAST) && IN_MULTICAST(ntohl(addr->s_addr))) {
+ DEBUG(SSSDBG_FUNC_DATA, ("Multicast IPv4 address %s\n", straddr));
+ return false;
+ } else if ((flags & SSS_NO_LOOPBACK)
+ && inet_netof(*addr) == IN_LOOPBACKNET) {
+ DEBUG(SSSDBG_FUNC_DATA, ("Loopback IPv4 address %s\n", straddr));
+ return false;
+ } else if ((flags & SSS_NO_LINKLOCAL)
+ && (addr->s_addr & htonl(0xffff0000)) == htonl(0xa9fe0000)) {
+ /* 169.254.0.0/16 */
+ DEBUG(SSSDBG_FUNC_DATA, ("Link-local IPv4 address %s\n", straddr));
+ return false;
+ } else if ((flags & SSS_NO_BROADCAST)
+ && addr->s_addr == htonl(INADDR_BROADCAST)) {
+ DEBUG(SSSDBG_FUNC_DATA, ("Broadcast IPv4 address %s\n", straddr));
+ return false;
+ }
+
+ return true;
+}
+
+bool check_ipv6_addr(struct in6_addr *addr, uint8_t flags)
+{
+ char straddr[INET6_ADDRSTRLEN];
+
+ if (inet_ntop(AF_INET6, addr, straddr, INET6_ADDRSTRLEN) == NULL) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ ("inet_ntop failed, won't log IP addresses\n"));
+ snprintf(straddr, INET6_ADDRSTRLEN, "unknown");
+ }
+
+ if ((flags & SSS_NO_LINKLOCAL) && IN6_IS_ADDR_LINKLOCAL(addr)) {
+ DEBUG(SSSDBG_FUNC_DATA, ("Link local IPv6 address %s\n", straddr));
+ return false;
+ } else if ((flags & SSS_NO_LOOPBACK) && IN6_IS_ADDR_LOOPBACK(addr)) {
+ DEBUG(SSSDBG_FUNC_DATA, ("Loopback IPv6 address %s\n", straddr));
+ return false;
+ } else if ((flags & SSS_NO_MULTICAST) && IN6_IS_ADDR_MULTICAST(addr)) {
+ DEBUG(SSSDBG_FUNC_DATA, ("Multicast IPv6 address %s\n", straddr));
+ return false;
+ }
+
+ return true;
+}
diff --git a/src/util/util.h b/src/util/util.h
index 20d230c..c84bc97 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -37,6 +37,7 @@
#include <pcre.h>
#include <sys/types.h>
#include <sys/stat.h>
+#include <arpa/inet.h>
#include <talloc.h>
#include <tevent.h>
@@ -438,6 +439,18 @@ errno_t check_and_open_readonly(const char *filename, int *fd, const uid_t uid,
enum check_file_type type);
/* from util.c */
+#define SSS_NO_LINKLOCAL 0x01
+#define SSS_NO_LOOPBACK 0x02
+#define SSS_NO_MULTICAST 0x04
+#define SSS_NO_BROADCAST 0x08
+
+#define SSS_NO_SPECIAL \
+ (SSS_NO_LINKLOCAL|SSS_NO_LOOPBACK|SSS_NO_MULTICAST|SSS_NO_BROADCAST)
+
+/* These two functions accept addr in network order */
+bool check_ipv4_addr(struct in_addr *addr, uint8_t check);
+bool check_ipv6_addr(struct in6_addr *addr, uint8_t check);
+
int split_on_separator(TALLOC_CTX *mem_ctx, const char *str,
const char sep, bool trim, bool skip_empty,
char ***_list, int *size);
--
1.7.11.2
>From 7d3f9b21746f272ad21879a0e24b69214291bf04 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzi...@redhat.com>
Date: Tue, 1 Oct 2013 16:36:55 +0200
Subject: [PATCH 2/4] dyndns: Use check_ipvX_addr functions
---
src/providers/dp_dyndns.c | 51 ++++++-----------------------------------------
1 file changed, 6 insertions(+), 45 deletions(-)
diff --git a/src/providers/dp_dyndns.c b/src/providers/dp_dyndns.c
index cd11431..77a128f 100644
--- a/src/providers/dp_dyndns.c
+++ b/src/providers/dp_dyndns.c
@@ -144,55 +144,16 @@ fail:
static bool
ok_for_dns(struct sockaddr *sa)
{
- char straddr[INET6_ADDRSTRLEN];
- struct in6_addr *addr6;
- struct in_addr *addr;
+ struct sockaddr_in sa4;
+ struct sockaddr_in6 sa6;
switch (sa->sa_family) {
case AF_INET6:
- addr6 = &((struct sockaddr_in6 *) sa)->sin6_addr;
-
- if (inet_ntop(AF_INET6, addr6, straddr, INET6_ADDRSTRLEN) == NULL) {
- DEBUG(SSSDBG_MINOR_FAILURE,
- ("inet_ntop failed, won't log IP addresses\n"));
- snprintf(straddr, INET6_ADDRSTRLEN, "unknown");
- }
-
- if (IN6_IS_ADDR_LINKLOCAL(addr6)) {
- DEBUG(SSSDBG_FUNC_DATA, ("Link local IPv6 address %s\n", straddr));
- return false;
- } else if (IN6_IS_ADDR_LOOPBACK(addr6)) {
- DEBUG(SSSDBG_FUNC_DATA, ("Loopback IPv6 address %s\n", straddr));
- return false;
- } else if (IN6_IS_ADDR_MULTICAST(addr6)) {
- DEBUG(SSSDBG_FUNC_DATA, ("Multicast IPv6 address %s\n", straddr));
- return false;
- }
- break;
+ memcpy(&sa6, sa, sizeof(struct sockaddr_in6));
+ return check_ipv6_addr(&sa6.sin6_addr, SSS_NO_SPECIAL);
case AF_INET:
- addr = &((struct sockaddr_in *) sa)->sin_addr;
-
- if (inet_ntop(AF_INET, addr, straddr, INET6_ADDRSTRLEN) == NULL) {
- DEBUG(SSSDBG_MINOR_FAILURE,
- ("inet_ntop failed, won't log IP addresses\n"));
- snprintf(straddr, INET6_ADDRSTRLEN, "unknown");
- }
-
- if (IN_MULTICAST(ntohl(addr->s_addr))) {
- DEBUG(SSSDBG_FUNC_DATA, ("Multicast IPv4 address %s\n", straddr));
- return false;
- } else if (inet_netof(*addr) == IN_LOOPBACKNET) {
- DEBUG(SSSDBG_FUNC_DATA, ("Loopback IPv4 address %s\n", straddr));
- return false;
- } else if ((addr->s_addr & htonl(0xffff0000)) == htonl(0xa9fe0000)) {
- /* 169.254.0.0/16 */
- DEBUG(SSSDBG_FUNC_DATA, ("Link-local IPv4 address %s\n", straddr));
- return false;
- } else if (addr->s_addr == htonl(INADDR_BROADCAST)) {
- DEBUG(SSSDBG_FUNC_DATA, ("Broadcast IPv4 address %s\n", straddr));
- return false;
- }
- break;
+ memcpy(&sa4, sa, sizeof(struct sockaddr_in));
+ return check_ipv4_addr(&sa4.sin_addr, SSS_NO_SPECIAL);
default:
DEBUG(SSSDBG_CRIT_FAILURE, ("Unknown address family\n"));
return false;
--
1.7.11.2
>From 9733fd17b3752997e567710b6a4ca10202489652 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzi...@redhat.com>
Date: Tue, 1 Oct 2013 17:06:38 +0200
Subject: [PATCH 3/4] sdap_async_sudo_hostinfo.c: Use check_ipvX_addr
---
src/providers/ldap/sdap_async_sudo_hostinfo.c | 62 +++++++++++----------------
1 file changed, 24 insertions(+), 38 deletions(-)
diff --git a/src/providers/ldap/sdap_async_sudo_hostinfo.c b/src/providers/ldap/sdap_async_sudo_hostinfo.c
index f0c7281..0deaff3 100644
--- a/src/providers/ldap/sdap_async_sudo_hostinfo.c
+++ b/src/providers/ldap/sdap_async_sudo_hostinfo.c
@@ -193,10 +193,10 @@ static int sdap_sudo_get_ip_addresses(TALLOC_CTX *mem_ctx,
char **ip_addr_list = NULL;
struct ifaddrs *ifaces = NULL;
struct ifaddrs *iface = NULL;
- struct sockaddr_in *ip4_addr = NULL;
- struct sockaddr_in *ip4_network = NULL;
- struct sockaddr_in6 *ip6_addr = NULL;
- struct sockaddr_in6 *ip6_network = NULL;
+ struct sockaddr_in ip4_addr;
+ struct sockaddr_in ip4_network;
+ struct sockaddr_in6 ip6_addr;
+ struct sockaddr_in6 ip6_network;
char ip_addr[INET6_ADDRSTRLEN + 1];
char network_addr[INET6_ADDRSTRLEN + 1];
in_addr_t ip4_netmask = 0;
@@ -230,55 +230,41 @@ static int sdap_sudo_get_ip_addresses(TALLOC_CTX *mem_ctx,
netmask = 0;
switch (iface->ifa_addr->sa_family) {
case AF_INET:
- ip4_addr = (struct sockaddr_in*)(iface->ifa_addr);
- ip4_network = (struct sockaddr_in*)(iface->ifa_netmask);
+ memcpy(&ip4_addr, iface->ifa_addr, sizeof(struct sockaddr_in));
+ memcpy(&ip4_network, iface->ifa_netmask, sizeof(struct sockaddr_in));
- /* ignore loopback */
- if (inet_netof(ip4_addr->sin_addr) == IN_LOOPBACKNET) {
- continue;
- }
-
- /* ignore multicast */
- if (IN_MULTICAST(ntohl(ip4_addr->sin_addr.s_addr))) {
- continue;
- }
-
- /* ignore broadcast */
- if (ntohl(ip4_addr->sin_addr.s_addr) == INADDR_BROADCAST) {
+ if (!check_ipv4_addr(&ip4_addr.sin_addr,
+ SSS_NO_LOOPBACK|SSS_NO_MULTICAST
+ |SSS_NO_BROADCAST)) {
continue;
}
/* get network mask length */
- ip4_netmask = ntohl(ip4_network->sin_addr.s_addr);
+ ip4_netmask = ntohl(ip4_network.sin_addr.s_addr);
while (ip4_netmask) {
netmask++;
ip4_netmask <<= 1;
}
/* get network address */
- ip4_network->sin_addr.s_addr = ip4_addr->sin_addr.s_addr
- & ip4_network->sin_addr.s_addr;
+ ip4_network.sin_addr.s_addr = ip4_addr.sin_addr.s_addr
+ & ip4_network.sin_addr.s_addr;
- sinx_addr = &ip4_addr->sin_addr;
- sinx_network = &ip4_network->sin_addr;
+ sinx_addr = &ip4_addr.sin_addr;
+ sinx_network = &ip4_network.sin_addr;
break;
case AF_INET6:
- ip6_addr = (struct sockaddr_in6*)(iface->ifa_addr);
- ip6_network = (struct sockaddr_in6*)(iface->ifa_netmask);
-
- /* ignore loopback */
- if (IN6_IS_ADDR_LOOPBACK(&ip6_addr->sin6_addr)) {
- continue;
- }
+ memcpy(&ip6_addr, iface->ifa_addr, sizeof(struct sockaddr_in6));
+ memcpy(&ip6_network, iface->ifa_netmask, sizeof(struct sockaddr_in6));
- /* ignore multicast */
- if (IN6_IS_ADDR_MULTICAST(&ip6_addr->sin6_addr)) {
+ if (!check_ipv6_addr(&ip6_addr.sin6_addr,
+ SSS_NO_LOOPBACK|SSS_NO_MULTICAST)) {
continue;
}
/* get network mask length */
for (i = 0; i < 4; i++) {
- ip6_netmask = ntohl(((uint32_t*)(&ip6_network->sin6_addr))[i]);
+ ip6_netmask = ntohl(((uint32_t*)(&ip6_network.sin6_addr))[i]);
while (ip6_netmask) {
netmask++;
ip6_netmask <<= 1;
@@ -287,13 +273,13 @@ static int sdap_sudo_get_ip_addresses(TALLOC_CTX *mem_ctx,
/* get network address */
for (i = 0; i < 4; i++) {
- ((uint32_t*)(&ip6_network->sin6_addr))[i] =
- ((uint32_t*)(&ip6_addr->sin6_addr))[i]
- & ((uint32_t*)(&ip6_network->sin6_addr))[i];
+ ((uint32_t*)(&ip6_network.sin6_addr))[i] =
+ ((uint32_t*)(&ip6_addr.sin6_addr))[i]
+ & ((uint32_t*)(&ip6_network.sin6_addr))[i];
}
- sinx_addr = &ip6_addr->sin6_addr;
- sinx_network = &ip6_network->sin6_addr;
+ sinx_addr = &ip6_addr.sin6_addr;
+ sinx_network = &ip6_network.sin6_addr;
break;
default:
/* skip other families */
--
1.7.11.2
>From 79122f704164d5de73cadd58a22b80a00c187f65 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzi...@redhat.com>
Date: Tue, 1 Oct 2013 18:37:22 +0200
Subject: [PATCH 4/4] tests: Silence alignment warning in tests.
---
src/tests/cmocka/test_dyndns.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/tests/cmocka/test_dyndns.c b/src/tests/cmocka/test_dyndns.c
index f819dc9..88e2c06 100644
--- a/src/tests/cmocka/test_dyndns.c
+++ b/src/tests/cmocka/test_dyndns.c
@@ -95,6 +95,7 @@ int __wrap_getifaddrs(struct ifaddrs **_ifap)
struct ifaddrs *ifap_head = NULL;
char *name;
char *straddr;
+ struct sockaddr_in *sa;
while ((name = sss_mock_ptr_type(char *)) != NULL) {
straddr = sss_mock_ptr_type(char *);
@@ -122,18 +123,21 @@ int __wrap_getifaddrs(struct ifaddrs **_ifap)
goto fail;
}
- ifap->ifa_addr = (struct sockaddr *) talloc(ifap, struct sockaddr_in);
- if (ifap->ifa_addr == NULL) {
+ /* Do not alocate directly on ifap->ifa_addr to
+ * avoid alignment warnings */
+ sa = talloc(ifap, struct sockaddr_in);
+ if (sa == NULL) {
errno = ENOMEM;
goto fail;
}
- ((struct sockaddr_in *) ifap->ifa_addr)->sin_family = AF_INET;
+ sa->sin_family = AF_INET;
/* convert straddr into ifa_addr */
- if (inet_pton(AF_INET, straddr,
- &(((struct sockaddr_in *) ifap->ifa_addr)->sin_addr)) != 1) {
+ if (inet_pton(AF_INET, straddr, &sa->sin_addr) != 1) {
goto fail;
}
+
+ ifap->ifa_addr = (struct sockaddr *) sa;
}
*_ifap = ifap_head;
--
1.7.11.2
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
