From a13784ba95efeb5a1f77253df40d433a1ce60087 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 12 Jun 2017 21:51:14 -0700 Subject: [PATCH] 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 Signed-off-by: Ben Pfaff Acked-by: Lance Richardson --- lib/byte-order.h | 21 +++++++++++++-------- lib/flow.c | 2 +- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/lib/byte-order.h b/lib/byte-order.h index e864658f9b5..b2a9082bbad 100644 --- 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 index 1f51b66e7b4..d73e796a2f4 100644 --- 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)) {