byte-order: Fix undefined behavior of BYTES_TO_BE32.

A left shift that would produce a result that is not representable by the type of the expression's result has "undefined behavior" according to the C language standard. Avoid this by casting values that could set the upper bit to unsigned types. Also document and convert a macro to a function. While we're at it, delete the unused macro BE16S_TO_BE32. Found via gcc's undefined behavior sanitizer. Reported-by: Lance Richardson <[email protected]> Signed-off-by: Ben Pfaff <[email protected]> Acked-by: Lance Richardson <[email protected]>
qlw · Jun 13, 2017 · a13784b · a13784b
1 parent 6b1d462
commit a13784b
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 9 deletions.
diff --git a/lib/byte-order.h b/lib/byte-order.h
@@ -98,17 +98,22 @@ uint32_byteswap(uint32_t crc) {
          ((((ovs_be64) (VALUE)) & UINT64_C(0xff00000000000000)) >> 56))
 #endif
 
+/* Returns the ovs_be32 that you would get from:
+ *
+ *    union { uint8_t b[4]; ovs_be32 be32; } x = { .b = { b0, b1, b2, b3 } };
+ *    return x.be32;
+ *
+ * but without the undefined behavior. */
+static inline ovs_be32
+bytes_to_be32(uint8_t b0, uint8_t b1, uint8_t b2, uint8_t b3)
+{
 #if WORDS_BIGENDIAN
-#define BYTES_TO_BE32(B1, B2, B3, B4) \
-    (OVS_FORCE ovs_be32)((uint32_t)(B1) << 24 | (B2) << 16 | (B3) << 8 | (B4))
-#define BE16S_TO_BE32(B1, B2) \
-    (OVS_FORCE ovs_be32)((uint32_t)(B1) << 16 | (B2))
+    uint32_t x = ((uint32_t) b0 << 24) | (b1 << 16) | (b2 << 8) | b3;
 #else
-#define BYTES_TO_BE32(B1, B2, B3, B4) \
-    (OVS_FORCE ovs_be32)((uint32_t)(B1) | (B2) << 8 | (B3) << 16 | (B4) << 24)
-#define BE16S_TO_BE32(B1, B2) \
-    (OVS_FORCE ovs_be32)((uint32_t)(B1) | (B2) << 16)
+    uint32_t x = ((uint32_t) b3 << 24) | (b2 << 16) | (b1 << 8) | b0;
 #endif
+    return (OVS_FORCE ovs_be32) x;
+}
 
 /* These functions zero-extend big-endian values to longer ones,
  * or truncate long big-endian value to shorter ones. */

diff --git a/lib/flow.c b/lib/flow.c
@@ -823,7 +823,7 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst)
 
     packet->l4_ofs = (char *)data - frame;
     miniflow_push_be32(mf, nw_frag,
-                       BYTES_TO_BE32(nw_frag, nw_tos, nw_ttl, nw_proto));
+                       bytes_to_be32(nw_frag, nw_tos, nw_ttl, nw_proto));
 
     if (OVS_LIKELY(!(nw_frag & FLOW_NW_FRAG_LATER))) {
         if (OVS_LIKELY(nw_proto == IPPROTO_TCP)) {