Avoid bitfields, unaligned accesses, packed structures, and PRI[ux]{16,32}.

Bitfields are not one of C's shining points.  There is *NO* guarantee in
what order bitfields are put within a structure - it's *NOT* necessarily
the same as the byte order of the machine, and it's *ESPECIALLY* not
guaranteed to be correlated with the value of the LBL_ALIGN definition
(that definition has to do with whether unaligned accesses are supported
by the hardware).  In addition, even if they're declared as unsigned,
that doesn't mean they're guaranteed to *be* unsigned.  Don't use them.

Unaligned accesses are not guaranteed to work, and fields in packets are
not guaranteed to be naturally aligned.  Use the EXTRACT_nBITS() macros.

__attribute((packed))__ is a GCCism, and is not guaranteed to be
supported by all compilers with which tcpdump can be compiled.  Make
integral fields > 1 byte arrays of u_int8_t's (which also lets us avoid
the & in the EXTRACT_nBITS() macros).

Some systems don't define the PRI[doux]16 and PRI[doux]32 macros, and
others define them infelicitously (i.e., for PRI[doux]32, with an "l";
our 32-bit integer types are *not* longs, as we don't care about
16-bit-"int" platforms).

diff --git a/mptcp.h b/mptcp.h
index 5980ad294ee5853d5d37297e85fe0df71f35a23f..4ff552e6935bcf302a80b551486438885cd018a5 100644 (file)
--- a/mptcp.h
+++ b/mptcp.h
@@ -45,120 +45,82 @@
 struct mptcp_option {
         u_int8_t        kind;
         u_int8_t        len;
-#if !LBL_ALIGN
-        u_int8_t        ver:4,
-                        sub:4;
-#else
-        u_int8_t        sub:4,
-                        ver:4;
-#endif
+        u_int8_t        sub_etc;        /* subtype upper 4 bits, other stuff lower 4 bits */
 };
 
+#define MPTCP_OPT_SUBTYPE(sub_etc)      (((sub_etc) >> 4) & 0xF)
+
 struct mp_capable {
         u_int8_t        kind;
         u_int8_t        len;
-#if !LBL_ALIGN
-        u_int8_t        ver:4,
-                        sub:4;
-        u_int8_t        s:1,
-                        rsv:6,
-                        c:1;
-#else
-        u_int8_t        sub:4,
-                        ver:4;
-        u_int8_t        c:1,
-                        rsv:6,
-                        s:1;
-#endif
-        u_int64_t        sender_key;
-        u_int64_t        receiver_key;
-} __attribute__((__packed__));
+        u_int8_t        sub_ver;
+        u_int8_t        flags;
+        u_int8_t        sender_key[8];
+        u_int8_t        receiver_key[8];
+};
+
+#define MP_CAPABLE_OPT_VERSION(sub_ver) (((sub_ver) >> 0) & 0xF)
+#define MP_CAPABLE_C                    0x80
+#define MP_CAPABLE_S                    0x01
 
 struct mp_join {
         u_int8_t        kind;
         u_int8_t        len;
-#if !LBL_ALIGN
-        u_int8_t        b:1,
-                        rsv:3,
-                        sub:4;
-#else
-        u_int8_t        sub:4,
-                        rsv:3,
-                        b:1;
-#endif
+        u_int8_t        sub_b;
         u_int8_t        addr_id;
         union {
                 struct {
-                        u_int32_t        token;
-                        u_int32_t        nonce;
+                        u_int8_t         token[4];
+                        u_int8_t         nonce[4];
                 } syn;
                 struct {
-                        u_int64_t        mac;
-                        u_int32_t        nonce;
+                        u_int8_t         mac[8];
+                        u_int8_t         nonce[4];
                 } synack;
                 struct {
                         u_int8_t        mac[20];
                 } ack;
         } u;
-} __attribute__((__packed__));
+};
+
+#define MP_JOIN_B                       0x01
 
 struct mp_dss {
         u_int8_t        kind;
         u_int8_t        len;
-#if !LBL_ALIGN
-        u_int16_t        rsv1:4,
-                        sub:4,
-                        A:1,
-                        a:1,
-                        M:1,
-                        m:1,
-                        F:1,
-                        rsv2:3;
-#else
-        u_int16_t        sub:4,
-                        rsv1:4,
-                        rsv2:3,
-                        F:1,
-                        m:1,
-                        M:1,
-                        a:1,
-                        A:1;
-#endif
+        u_int8_t        sub;
+        u_int8_t        flags;
 };
 
+#define MP_DSS_F                        0x10
+#define MP_DSS_m                        0x08
+#define MP_DSS_M                        0x04
+#define MP_DSS_a                        0x02
+#define MP_DSS_A                        0x01
+
 struct mp_add_addr {
         u_int8_t        kind;
         u_int8_t        len;
-#if !LBL_ALIGN
-        u_int8_t        ipver:4,
-                        sub:4;
-#else
-        u_int8_t        sub:4,
-                        ipver:4;
-#endif
+        u_int8_t        sub_ipver;
         u_int8_t        addr_id;
         union {
                 struct {
-                        struct in_addr   addr;
-                        u_int16_t        port;
+                        u_int8_t         addr[4];
+                        u_int8_t         port[2];
                 } v4;
                 struct {
-                        struct in6_addr  addr;
-                        u_int16_t        port;
+                        u_int8_t         addr[16];
+                        u_int8_t         port[2];
                 } v6;
         } u;
-} __attribute__((__packed__));
+};
+
+#define MP_ADD_ADDR_IPVER(sub_ipver)    (((sub_ipver) >> 0) & 0xF)
 
 struct mp_remove_addr {
         u_int8_t        kind;
         u_int8_t        len;
-#if !LBL_ALIGN
-        u_int8_t        rsv:4,
-                        sub:4;
-#else
-        u_int8_t        sub:4,
-                        rsv:4;
-#endif
+        u_int8_t        sub;
         /* list of addr_id */
         u_int8_t        addrs_id;
 };
@@ -166,45 +128,24 @@ struct mp_remove_addr {
 struct mp_fail {
         u_int8_t        kind;
         u_int8_t        len;
-#if !LBL_ALIGN
-        u_int16_t       rsv1:4,
-                        sub:4,
-                        rsv2:8;
-#else
-        u_int16_t       sub:4,
-                        rsv1:4,
-                        rsv2:8;
-#endif
-        u_int64_t        data_seq;
-} __attribute__((__packed__));
-
-struct mp_fclose {
+        u_int8_t        sub;
+        u_int8_t        resv;
+        u_int8_t        data_seq[8];
+};
+
+struct mp_close {
         u_int8_t        kind;
         u_int8_t        len;
-#if !LBL_ALIGN
-        u_int16_t       rsv1:4,
-                        sub:4,
-                        rsv2:8;
-#else
-        u_int16_t       sub:4,
-                        rsv1:4,
-                        rsv2:8;
-#endif
-        u_int64_t        key;
-} __attribute__((__packed__));
+        u_int8_t        sub;
+        u_int8_t        rsv;
+        u_int8_t        key[8];
+};
 
 struct mp_prio {
         u_int8_t        kind;
         u_int8_t        len;
-#if !LBL_ALIGN
-        u_int8_t        b:1,
-                        rsv:3,
-                        sub:4;
-#else
-        u_int8_t        sub:4,
-                        rsv:3,
-                        b:1;
-#endif
+        u_int8_t        sub_b;
         u_int8_t        addr_id;
-} __attribute__((__packed__));
+};
 
+#define MP_PRIO_B                       0x01

diff --git a/print-mptcp.c b/print-mptcp.c
index edcfe0797d31b0f1bc4013911e5c1e30ed4f6b03..c5fad69b74afcd01dc38e6b300fd9ce552aef8ad 100644 (file)
--- a/print-mptcp.c
+++ b/print-mptcp.c
@@ -62,16 +62,16 @@ static int mp_capable_print(const u_char *opt, u_int opt_len, u_char flags)
             !(opt_len == 20 && (flags & (TH_SYN | TH_ACK)) == TH_ACK))
                 return 0;
 
-        if (mpc->ver != 0) {
-                printf(" Unknown Version (%d)", mpc->ver);
+        if (MP_CAPABLE_OPT_VERSION(mpc->sub_ver) != 0) {
+                printf(" Unknown Version (%d)", MP_CAPABLE_OPT_VERSION(mpc->sub_ver));
                 return 1;
         }
 
-        if (mpc->c)
+        if (mpc->flags & MP_CAPABLE_C)
                 printf(" csum");
-        printf(" {0x%" PRIx64, EXTRACT_64BITS(&mpc->sender_key));
+        printf(" {0x%" PRIx64, EXTRACT_64BITS(mpc->sender_key));
         if (opt_len == 20) /* ACK */
-                printf(",0x%" PRIx64, EXTRACT_64BITS(&mpc->receiver_key));
+                printf(",0x%" PRIx64, EXTRACT_64BITS(mpc->receiver_key));
         printf("}");
         return 1;
 }
@@ -86,21 +86,21 @@ static int mp_join_print(const u_char *opt, u_int opt_len, u_char flags)
                 return 0;
 
         if (opt_len != 24) {
-                if (mpj->b)
+                if (mpj->sub_b & MP_JOIN_B)
                         printf(" backup");
                 printf(" id %u", mpj->addr_id);
         }
 
         switch (opt_len) {
         case 12: /* SYN */
-                printf(" token 0x%" PRIx32 " nonce 0x%" PRIx32,
-                        EXTRACT_32BITS(&mpj->u.syn.token),
-                        EXTRACT_32BITS(&mpj->u.syn.nonce));
+                printf(" token 0x%x" " nonce 0x%x",
+                        EXTRACT_32BITS(mpj->u.syn.token),
+                        EXTRACT_32BITS(mpj->u.syn.nonce));
                 break;
         case 16: /* SYN/ACK */
-                printf(" hmac 0x%" PRIx64 " nonce 0x%" PRIx32,
-                        EXTRACT_64BITS(&mpj->u.synack.mac),
-                        EXTRACT_32BITS(&mpj->u.synack.nonce));
+                printf(" hmac 0x%" PRIx64 " nonce 0x%x",
+                        EXTRACT_64BITS(mpj->u.synack.mac),
+                        EXTRACT_32BITS(mpj->u.synack.nonce));
                 break;
         case 24: {/* ACK */
                 size_t i;
@@ -116,7 +116,30 @@ static int mp_join_print(const u_char *opt, u_int opt_len, u_char flags)
 
 static u_int mp_dss_len(struct mp_dss *m, int csum)
 {
-        return 4 + m->A * (4 + m->a * 4) + m->M * (10 + m->m * 4 + csum * 2);
+        u_int len;
+
+        len = 4;
+        if (m->flags & MP_DSS_A) {
+                /* Ack present - 4 or 8 octets */
+                len += (m->flags & MP_DSS_a) ? 8 : 4;
+        }
+        if (m->flags & MP_DSS_M) {
+                /*
+                 * Data Sequence Number (DSN), Subflow Sequence Number (SSN),
+                 * Data-Level Length present, and Checksum possibly present.
+                 * All but the Checksum are 10 bytes if the m flag is
+                 * clear (4-byte DSN) and 14 bytes if the m flag is set
+                 * (8-byte DSN).
+                 */
+                len += (m->flags & MP_DSS_m) ? 14 : 10;
+
+                /*
+                 * The Checksum is present only if negotiated.
+                 */
+                if (csum)
+                        len += 2;
+       }
+       return len;
 }
 
 static int mp_dss_print(const u_char *opt, u_int opt_len, u_char flags)
@@ -127,33 +150,37 @@ static int mp_dss_print(const u_char *opt, u_int opt_len, u_char flags)
              opt_len != mp_dss_len(mdss, 0)) || flags & TH_SYN)
                 return 0;
 
-        if (mdss->F)
+        if (mdss->flags & MP_DSS_F)
                 printf(" fin");
 
         opt += 4;
-        if (mdss->A) {
+        if (mdss->flags & MP_DSS_A) {
                 printf(" ack ");
-                if (mdss->a)
+                if (mdss->flags & MP_DSS_a) {
                         printf("%" PRIu64, EXTRACT_64BITS(opt));
-                else
-                        printf("%" PRIu32, EXTRACT_32BITS(opt));
-                opt += mdss->a ? 8 : 4;
+                        opt += 8;
+                } else {
+                        printf("%u", EXTRACT_32BITS(opt));
+                        opt += 4;
+                }
         }
 
-        if (mdss->M) {
+        if (mdss->flags & MP_DSS_M) {
                 printf(" seq ");
-                if (mdss->m)
+                if (mdss->flags & MP_DSS_m) {
                         printf("%" PRIu64, EXTRACT_64BITS(opt));
-                else
-                        printf("%" PRIu32, EXTRACT_32BITS(opt));
-                opt += mdss->m ? 8 : 4;
-                printf(" subseq %" PRIu32, EXTRACT_32BITS(opt));
+                        opt += 8;
+                } else {
+                        printf("%u", EXTRACT_32BITS(opt));
+                        opt += 4;
+                }
+                printf(" subseq %u", EXTRACT_32BITS(opt));
                 opt += 4;
-                printf(" len %" PRIu16, EXTRACT_16BITS(opt));
+                printf(" len %u", EXTRACT_16BITS(opt));
                 opt += 2;
 
                 if (opt_len == mp_dss_len(mdss, 1))
-                        printf(" csum 0x%" PRIx16, EXTRACT_16BITS(opt));
+                        printf(" csum 0x%x", EXTRACT_16BITS(opt));
         }
         return 1;
 }
@@ -161,29 +188,30 @@ static int mp_dss_print(const u_char *opt, u_int opt_len, u_char flags)
 static int add_addr_print(const u_char *opt, u_int opt_len, u_char flags _U_)
 {
         struct mp_add_addr *add_addr = (struct mp_add_addr *) opt;
+        u_int ipver = MP_ADD_ADDR_IPVER(add_addr->sub_ipver);
 
-        if (!((opt_len == 8 || opt_len == 10) && add_addr->ipver == 4) &&
-            !((opt_len == 20 || opt_len == 22) && add_addr->ipver == 6))
+        if (!((opt_len == 8 || opt_len == 10) && ipver == 4) &&
+            !((opt_len == 20 || opt_len == 22) && ipver == 6))
                 return 0;
 
         printf(" id %u", add_addr->addr_id);
-        switch (add_addr->ipver) {
+        switch (ipver) {
         case 4:
-                printf(" %s", ipaddr_string(&add_addr->u.v4.addr));
+                printf(" %s", ipaddr_string(add_addr->u.v4.addr));
+                if (opt_len == 10)
+                        printf(":%u", EXTRACT_16BITS(add_addr->u.v4.port));
                 break;
         case 6:
 #ifdef INET6
-                printf(" %s", ip6addr_string(&add_addr->u.v6.addr));
+                printf(" %s", ip6addr_string(add_addr->u.v6.addr));
 #endif
+                if (opt_len == 22)
+                        printf(":%u", EXTRACT_16BITS(add_addr->u.v6.port));
                 break;
         default:
                 return 0;
         }
 
-        if (opt_len == 10 || opt_len == 22)
-                printf(":%" PRIu16, ntohs(add_addr->ipver == 4 ?
-                                         add_addr->u.v4.port :
-                                         add_addr->u.v6.port));
         return 1;
 }
 
@@ -209,7 +237,7 @@ static int mp_prio_print(const u_char *opt, u_int opt_len, u_char flags _U_)
         if (opt_len != 3 && opt_len != 4)
                 return 0;
 
-        if (mpp->b)
+        if (mpp->sub_b & MP_PRIO_B)
                 printf(" backup");
         else
                 printf(" non-backup");
@@ -261,7 +289,7 @@ int mptcp_print(const u_char *cp, u_int len, u_char flags)
                 return 0;
 
         opt = (struct mptcp_option *) cp;
-        subtype = min(opt->sub, MPTCP_SUB_FCLOSE + 1);
+        subtype = min(MPTCP_OPT_SUBTYPE(opt->sub_etc), MPTCP_SUB_FCLOSE + 1);
 
         printf(" %s", mptcp_options[subtype].name);
         return mptcp_options[subtype].print(cp, len, flags);