diff mbox series

[ovs-dev,v5,1/6] treewide: Fix invalid bit shift operations.

Message ID 20220405124226.16152.88197.stgit@dceara.remote.csb
State Superseded
Headers show
Series Fix UndefinedBehaviorSanitizer reported issues and enable it in CI. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Dumitru Ceara April 5, 2022, 12:42 p.m. UTC
UB Sanitizer reports:
  tests/test-hash.c:59:40: runtime error: shift exponent 64 is too large for 64-bit type 'long unsigned int'
      #0 0x44c3c9 in get_range128 tests/test-hash.c:59
      #1 0x44cb2e in check_hash_bytes128 tests/test-hash.c:178
      #2 0x44d14d in test_hash_main tests/test-hash.c:282
      [...]
  ofproto/ofproto-dpif-xlate.c:5607:45: runtime error: left shift of 65535 by 16 places cannot be represented in type 'int'
      #0 0x53fe9f in xlate_sample_action ofproto/ofproto-dpif-xlate.c:5607
      #1 0x54d625 in do_xlate_actions ofproto/ofproto-dpif-xlate.c:7160
      #2 0x553b76 in xlate_actions ofproto/ofproto-dpif-xlate.c:7806
      #3 0x4fcb49 in upcall_xlate ofproto/ofproto-dpif-upcall.c:1237
      #4 0x4fe02f in process_upcall ofproto/ofproto-dpif-upcall.c:1456
      #5 0x4fda99 in upcall_cb ofproto/ofproto-dpif-upcall.c:1358
      [...]
  tests/test-util.c:89:23: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'
      #0 0x476415 in test_ctz tests/test-util.c:89
      [...]
  lib/dpif-netlink.c:396:33: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'
      #0 0x571b9f in dpif_netlink_open lib/dpif-netlink.c:396

Acked-by: Aaron Conole <aconole@redhat.com>
Acked-by: Paolo Valerio <pvalerio@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
---
v5:
- Rebase.
v4:
- Added Paolo's ack.
- Addressed Ilya's comments.
v3:
- Added Aaron's ack.
---
 lib/dpif-netlink.c           |    2 +-
 ofproto/ofproto-dpif-xlate.c |    3 ++-
 tests/test-hash.c            |    3 +++
 tests/test-util.c            |   13 ++++++-------
 4 files changed, 12 insertions(+), 9 deletions(-)
diff mbox series

Patch

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 71e35ccddaae..06e1e8ca0283 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -85,7 +85,7 @@  enum { MAX_PORTS = USHRT_MAX };
 #define EPOLLEXCLUSIVE (1u << 28)
 #endif
 
-#define OVS_DP_F_UNSUPPORTED (1 << 31);
+#define OVS_DP_F_UNSUPPORTED (1u << 31);
 
 /* This PID is not used by the kernel datapath when using dispatch per CPU,
  * but it is required to be set (not zero). */
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index bfd4960ddb8f..792d9a3cd0eb 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -5641,7 +5641,8 @@  xlate_sample_action(struct xlate_ctx *ctx,
 
     /* Scale the probability from 16-bit to 32-bit while representing
      * the same percentage. */
-    uint32_t probability = (os->probability << 16) | os->probability;
+    uint32_t probability =
+        ((uint32_t) os->probability << 16) | os->probability;
 
     /* If ofp_port in flow sample action is equel to ofp_port,
      * this sample action is a input port action. */
diff --git a/tests/test-hash.c b/tests/test-hash.c
index 5d3f8ea43f65..aec5f580bb75 100644
--- a/tests/test-hash.c
+++ b/tests/test-hash.c
@@ -55,6 +55,9 @@  set_bit128(ovs_u128 *values, int bit, int n_bits)
 static uint64_t
 get_range128(ovs_u128 *value, int ofs, uint64_t mask)
 {
+    if (ofs == 0) {
+        return value->u64.lo & mask;
+    }
     return ((ofs < 64 ? (value->u64.lo >> ofs) : 0) & mask)
         | ((ofs <= 64 ? (value->u64.hi << (64 - ofs)) : (value->u64.hi >> (ofs - 64)) & mask));
 }
diff --git a/tests/test-util.c b/tests/test-util.c
index f0fd0421086b..7d899fbbfd92 100644
--- a/tests/test-util.c
+++ b/tests/test-util.c
@@ -43,17 +43,16 @@  check_log_2_floor(uint32_t x, int n)
 static void
 test_log_2_floor(struct ovs_cmdl_context *ctx OVS_UNUSED)
 {
-    int n;
-
-    for (n = 0; n < 32; n++) {
+    for (uint32_t n = 0; n < 32; n++) {
         /* Check minimum x such that f(x) == n. */
-        check_log_2_floor(1 << n, n);
+        check_log_2_floor(UINT32_C(1) << n, n);
 
         /* Check maximum x such that f(x) == n. */
-        check_log_2_floor((1 << n) | ((1 << n) - 1), n);
+        check_log_2_floor((UINT32_C(1) << n) | ((UINT32_C(1) << n) - 1), n);
 
         /* Check a random value in the middle. */
-        check_log_2_floor((random_uint32() & ((1 << n) - 1)) | (1 << n), n);
+        check_log_2_floor((random_uint32() & ((UINT32_C(1) << n) - 1))
+                          | (UINT32_C(1) << n), n);
     }
 
     /* log_2_floor(0) is undefined, so don't check it. */
@@ -86,7 +85,7 @@  test_ctz(struct ovs_cmdl_context *ctx OVS_UNUSED)
 
     for (n = 0; n < 32; n++) {
         /* Check minimum x such that f(x) == n. */
-        check_ctz32(1 << n, n);
+        check_ctz32(UINT32_C(1) << n, n);
 
         /* Check maximum x such that f(x) == n. */
         check_ctz32(UINT32_MAX << n, n);