OpenFlow: Add an overshoot guard back.

In openflow_print() terminate the loop if the downstream code managed
to run off the TCP payload end without running off the packet buffer
end (that's how the pointers worked before commit ad69daa2, but I got
the recent conversion to a decrementing unsigned length wrong in commit
4e2e9c24). Expand the comment further.

Declare OF_HEADER_FIXLEN as unsigned to squelch a signedness warning.

diff --git a/openflow.h b/openflow.h
index 4c2d9a7a65fdd91cff973577406c2890dd632218..f35648a5d7890ecfdd25bb251d93e919992c3e49 100644 (file)
--- a/openflow.h
+++ b/openflow.h
@@ -41,7 +41,7 @@
        len -= (n); \
 }
 
-#define OF_HEADER_FIXLEN 8
+#define OF_HEADER_FIXLEN 8U
 
 #define ONF_EXP_ONF               0x4f4e4600
 #define ONF_EXP_BUTE              0xff000001

diff --git a/print-openflow.c b/print-openflow.c
index 87a68a47cafef11484f19de94c4c40ec5bafecb9..3b85dd7a9e7c369cd2e25fd14c90753948f24358 100644 (file)
--- a/print-openflow.c
+++ b/print-openflow.c
@@ -101,6 +101,11 @@ openflow_print(netdissect_options *ndo, const u_char *cp, u_int len)
                xid = GET_BE_U_4(cp);
                OF_FWD(4);
                /*
+                * When a TCP packet can contain several protocol messages,
+                * and at the same time a protocol message can span several
+                * TCP packets, decoding an incomplete message at the end of
+                * a TCP packet requires attention to detail in this loop.
+                *
                 * Message length includes the header length and a message
                 * always includes the basic header. A message length underrun
                 * fails decoding of the rest of the current packet. At the
@@ -108,11 +113,18 @@ openflow_print(netdissect_options *ndo, const u_char *cp, u_int len)
                 * possible even when it does not end within the current TCP
                 * segment.
                 *
-                * That is, do NOT require the header "length" to be small
+                * Specifically, to try to process the message body in this
+                * iteration do NOT require the header "length" to be small
                 * enough for the full declared OpenFlow message to fit into
                 * the remainder of the declared TCP segment, same as the full
                 * declared TCP segment is not required to fit into the
                 * captured packet buffer.
+                *
+                * But DO require the same at the end of this iteration to
+                * decrement "len" and to proceed to the next iteration.
+                * (Ideally the declared TCP payload end will be at or after
+                * the captured packet buffer end, but stay safe even when
+                * that's somehow not the case.)
                 */
                if (length < OF_HEADER_FIXLEN) {
                        of_header_print(ndo, version, type, length, xid);
@@ -131,6 +143,8 @@ openflow_print(netdissect_options *ndo, const u_char *cp, u_int len)
                        of_header_print(ndo, version, type, length, xid);
                        ND_TCHECK_LEN(cp, length - OF_HEADER_FIXLEN);
                }
+               if (length - OF_HEADER_FIXLEN > len)
+                       break;
                OF_FWD(length - OF_HEADER_FIXLEN);
        } /* while (len) */
        return;