NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

kern/44403: [dM] TUN_PREPADDR broken for IPv6

>Number:         44403
>Category:       kern
>Synopsis:       TUN_PREPADDR works only with AF_INET
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jan 17 09:45:00 +0000 2011
>Originator:     der Mouse
>Release:        NetBSD 4.0.1
        Any 4.0.1, likely other versions too.
        When TUN_PREPADDR is turned on on a tun interface (which means
        using the TUNSLMODE ioctl on the control device), it silently
        stops passing anything but AF_INET packets.  In particular, it
        drops AF_INET6 packets.

        Careful code inspection reveals what's going on: the control
        structure in tun_output causes the "drop if not INET" test,
        which is carefully skipped if TUN_IFHEAD is set, to be
        performed when TUN_PREPADDR is set.

        In my case, this broke my house IPv6 connectivity when I moved
        one end of the tunnel which provides its v6 uplink to 4.0.1
        (from 3.1, into the if_tun.c of which I had hacked more or less
        the same PREPADDR support which appears in stock 4.0.1, only
        without this bug, because I didn't implement TUN_IFHEAD there).
        Turn on TUN_PREPADDR with TUNSLMODE and watch your v6 traffic
        silently vanish before the userland process even sees it.  This
        makes TUN_PREPADDR substantially less useful than its design
        purpose (which was to provide not only the address family, as
        TUN_IFHEAD does, but the rest of the destination address as
        well, thus making TUNSIFMODE specifying IFF_BROADCAST useful).
        This works for me:

        --- OLD/if_tun.c        2011-01-17 04:19:59.000000000 -0500
        +++ NEW/if_tun.c        2011-01-17 04:20:30.000000000 -0500
        @@ -496,6 +496,7 @@
         #if defined(INET) || defined(INET6)
                int             mlen;
                uint32_t        *af;
        +       int             non_inet_ok;
                ALTQ_DECL(struct altq_pktattr pktattr;)
        @@ -530,6 +531,7 @@
                case AF_INET:
         #if defined(INET) || defined(INET6)
        +               non_inet_ok = 0;
                        if (tp->tun_flags & TUN_PREPADDR) {
                                /* Simple link-layer header */
                                M_PREPEND(m0, dst->sa_len, M_DONTWAIT);
        @@ -539,8 +541,8 @@
                                        goto out;
                                bcopy(dst, mtod(m0, char *), dst->sa_len);
        +                       non_inet_ok = 1;
                        if (tp->tun_flags & TUN_IFHEAD) {
                                /* Prepend the address family */
                                M_PREPEND(m0, sizeof(*af), M_DONTWAIT);
        @@ -551,8 +553,10 @@
                                af = mtod(m0,uint32_t *);
                                *af = htonl(dst->sa_family);
        -               } else {
        -#ifdef INET     
        +                       non_inet_ok = 1;
        +               }
        +               if (! non_inet_ok) {
        +#ifdef INET
                                if (dst->sa_family != AF_INET)

        It should be enough to just add an "else" before the if that
        tests TUN_IFHEAD, because TUN_PREADDR and TUN_IFHEAD should
        never both be set (when setting either, the other is cleared).
        However, this strikes me as more robust, mostly in case someone
        someday decides to allow them to both be set.  It also makes
        what is going on more explicit to a reader of the code.

/~\ The ASCII                             Mouse
\ / Ribbon Campaign
 X  Against HTML      
/ \ Email!           7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B

Home | Main Index | Thread Index | Old Index