[distcc] [PATCH] IPv6 ACLs

[Fergus Henderson]

Thanks for the patch!

Some comments:

+        host_str = malloc(value_len + 1);
+        strncpy(host_str, spec, value_len);

Here you are not checking the return value of malloc().

Oh, I see that the old code that this is replacing didn't check the return
value of strdup() either.
So I guess this is no worse.

+static void set_allones(dcc_address_t *addr)
+{
+#ifndef ENABLE_RFC2553
+    *addr = *(dcc_address_t *)&allones;

I'm concerned that this code may violate the C standard's rules on
aliasing/type-punning.

+static void set_mask_inet6(struct in6_addr *mask, int mask_bits) {
+    uint32_t *iptr = (uint32_t *)mask;
+    int allones_count = mask_bits / 32;
+    int i;
+
+    for(i = 0; i < allones_count; ++i, ++iptr)
+        *iptr = allones;
+
+    set_mask_32(iptr, mask_bits % 32);
+    ++iptr;
+
+    for(i = 4 - allones_count - 1; i > 0; --i, ++iptr)
+        *iptr = 0;
+}

Likewise here.

The C standard says: (ISO/IEC 9899:TC2, section 6.5, paragraph 7):

An object shall have its stored value accessed only by an lvalue expression
that has one of
the following types:74)
-- a type compatible with the effective type of the object,
-- a qualified version of a type compatible with the effective type of the
object,
-- a type that is the signed or unsigned type corresponding to the effective
type of the
    object,
-- a type that is the signed or unsigned type corresponding to a qualified
version of the
    effective type of the object,
-- an aggregate or union type that includes one of the aforementioned types
among its
    members (including, recursively, a member of a subaggregate or contained
union), or
-- a character type.

The Posix standard says:

[IP6 <javascript:open_code('IP6')>] [image: [Option Start]] The *
<netinet/in.h>* header shall define the *in6_addr* structure that contains
at least the following member:

uint8_t s6_addr[16]

So in general I don't think it is standard-conforming to assume that an
in6_addr can be treated as an array of uint32_t.

I think that this issue could be avoided by rewriting the code using uint8_t
instead of uint32_t and using the s6_addr member of the in6_addr struct
instead of casts.

Now in practice the code probably works fine.  So maybe I'm being a bit
pedantic here.
But I think it could be made standard-conforming without that much
additional effort.
What do you think?

Cheers,
  Fergus.

On Mon, Jul 13, 2009 at 4:52 AM, Bob Ham <rah at bash.sh> wrote:

> Hi there,
>
> The attached patch implements IPv6 support for access control.  It also
> includes a small update to restrict zeroconf advertisements to IPv4 if
> RFC2553 support isn't enabled.
>
> I've tested the patch both with RFC2553 support enabled and without.
> There seem to be no regressions.
>
> A more extensive zeroconf update will follow to restrict the addresses
> it advertises to ones that are actually listened on.  It seems there may
> be issues listening on IPv6 addresses as well.
>
> Regards,
>
> Bob Ham
>
> --
> Bob Ham <rah at bash.sh>
>
> for (;;) { ++pancakes; }
>
> __
> distcc mailing list            http://distcc.samba.org/
> To unsubscribe or change options:
> https://lists.samba.org/mailman/listinfo/distcc
>



-- 
Fergus Henderson <fergus at google.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.samba.org/pipermail/distcc/attachments/20090720/142c06e6/attachment.html>