On Fri, Sep 13, 2013 at 09:48:33AM +0200, Lukas Slebodnik wrote:
> On (13/09/13 09:29), Lukas Slebodnik wrote:
> >On (12/09/13 18:47), Jakub Hrozek wrote:
> >>The attached patch fixes
> >>https://bugzilla.redhat.com/show_bug.cgi?id=1007475#c2
> >
> >>From eaadcee0fc1335e7c37b2c04ae4fb39fe7a58b59 Mon Sep 17 00:00:00 2001
> >>From: Jakub Hrozek <[email protected]>
> >>Date: Thu, 12 Sep 2013 18:45:54 +0200
> >>Subject: [PATCH] Convert IN_MULTICAST parameter to host order
> >>
> >>https://fedorahosted.org/sssd/ticket/2087
> >>
> >>IN_MULTICAST accepts address in the host order, but network order was
> >>supplied.
> >>---
> >> src/monitor/monitor_netlink.c | 2 +-
> >> src/providers/ldap/sdap_async_sudo_hostinfo.c | 2 +-
> >> 2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/src/monitor/monitor_netlink.c b/src/monitor/monitor_netlink.c
> >>index
> >>c013423780f318f2a0f12dd5fa50babe12cdcd18..6baf13652b3c42ad92669272f262ac8b59450efe
> >> 100644
> >>--- a/src/monitor/monitor_netlink.c
> >>+++ b/src/monitor/monitor_netlink.c
> >>@@ -610,7 +610,7 @@ static bool route_is_multicast(struct rtnl_route
> >>*route_obj)
> >> return false;
> >> }
> >>
> >>- return IN_MULTICAST(addr4->s_addr);
> >>+ return IN_MULTICAST(ntohl(addr4->s_addr));
> >> } else if (nl_addr_get_family(nl) == AF_INET6) {
> >> addr6 = nl_addr_get_binary_addr(nl);
> >> if (!addr6) {
> >>diff --git a/src/providers/ldap/sdap_async_sudo_hostinfo.c
> >>b/src/providers/ldap/sdap_async_sudo_hostinfo.c
> >>index
> >>4e33babd505dd218ddfd37af21e62fb0bcbe451c..f0c728108f19d965c4b1f07f1067d6862fd0c371
> >> 100644
> >>--- a/src/providers/ldap/sdap_async_sudo_hostinfo.c
> >>+++ b/src/providers/ldap/sdap_async_sudo_hostinfo.c
> >>@@ -239,7 +239,7 @@ static int sdap_sudo_get_ip_addresses(TALLOC_CTX
> >>*mem_ctx,
> >> }
> >>
> >> /* ignore multicast */
> >>- if (IN_MULTICAST(ip4_addr->sin_addr.s_addr)) {
> >>+ if (IN_MULTICAST(ntohl(ip4_addr->sin_addr.s_addr))) {
> >> continue;
> >> }
> >
> >IN_MULTICAST(a) is defined as
> >((((in_addr_t)(a)) & 0xf0000000) == 0xe0000000)
> >
> >Therefore "argument a" have to be in host byte order, because
> >it dost not make sense to do a bitwise operation
> >with numbers of different byte orders.
> >
> >With this patch, ntohl is used in each macro IN_MULTICAST.
> >
> >bash$ git grep -n IN_MULTICAST
> >src/monitor/monitor_netlink.c:613: return
> >IN_MULTICAST(ntohl(addr4->s_addr));
> >src/providers/dp_dyndns.c:181: if (IN_MULTICAST(ntohl(addr->s_addr)))
> >{
> >src/providers/ldap/sdap_async_sudo_hostinfo.c:242: if
> >(IN_MULTICAST(ntohl(ip4_addr->sin_addr.
> >
> >ACK
> >
> >LS
> >_______________________________________________
> >sssd-devel mailing list
> >[email protected]
> >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
> src/providers/dp_dyndns.c
> 187 } else if ((addr->s_addr & 0xffff0000) == 0xa9fe0000) {
> ^^^^^^^^^^^^ ^^^^^^^^^^
> net order host order
> comparision is wrong
> for example ip "192.168.254.169" can return false
> We should use:
> --htonl for constant 0xffff0000
> --ntohl for addr->s_addr
>
> 188 /* 169.254.0.0/16 */
> 189 DEBUG(SSSDBG_FUNC_DATA, ("Link-local IPv4 address %s\n",
> straddr));
> 190 return false;
> 191 } else if (addr->s_addr == htonl(INADDR_BROADCAST)) {
> ^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^
> net order net order
> comparision is fine
> 192 DEBUG(SSSDBG_FUNC_DATA, ("Broadcast IPv4 address %s\n",
> straddr));
> 193 return false;
> 194 }
>
>
>
>
> I hope that I checked all important files, but someone else should take a
> look.
>
> LS
Thank you, I think I should add a unit test as well.
Btw I also checked how are the IPv6 macros defined and I think we're
fine. Some implementations of IN6_IS_ADDR_MULTICAST are defined as:
int IN6_IS_ADDR_MULTICAST(const struct in6_addr *a)
{
return (a->s6_bytes[0] == 0xff);
}
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
