diff mbox series

[ovs-dev,v3,09/14] ofp-actions: Ensure aligned accesses when setting masked fields.

Message ID 20220124141916.11777.50034.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

Commit Message

Dumitru Ceara Jan. 24, 2022, 2:19 p.m. UTC 
For example is parsing the OVN "eth.dst[40] = 1;" action, which
triggered the following warning from UndefinedBehaviorSanitizer:

  lib/meta-flow.c:3210:9: runtime error: member access within misaligned address 0x000000de4e36 for type 'const union mf_value', which requires 8 byte alignment
  0x000000de4e36: note: pointer points here
   00 00 00 00 01 00  00 00 00 00 00 00 00 00  70 4e de 00 00 00 00 00  10 51 de 00 00 00 00 00  c0 4f
               ^
      #0 0x5818bc in mf_format lib/meta-flow.c:3210
      #1 0x5b6047 in format_SET_FIELD lib/ofp-actions.c:3342
      #2 0x5d68ab in ofpact_format lib/ofp-actions.c:9213
      #3 0x5d6ee0 in ofpacts_format lib/ofp-actions.c:9237
      #4 0x410922 in test_parse_actions tests/test-ovn.c:1360

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
---
v3: Split out from old patch 07/11.
---
 include/openvswitch/util.h   |    3 +++
 lib/meta-flow.c              |    6 ++++++
 lib/nx-match.c               |    7 +++++++
 ofproto/ofproto-dpif-xlate.c |   13 +++++++++----
 4 files changed, 25 insertions(+), 4 deletions(-)
diff mbox series

Patch

diff --git a/include/openvswitch/util.h b/include/openvswitch/util.h
index 228b185c3a5f..f45dc505164c 100644
--- a/include/openvswitch/util.h
+++ b/include/openvswitch/util.h
@@ -285,6 +285,9 @@  is_pow2(uintmax_t x)
  * segfault, so it is important to be aware of correct alignment. */
 #define ALIGNED_CAST(TYPE, ATTR) ((TYPE) (void *) (ATTR))
 
+#define IS_PTR_ALIGNED(OBJ) \
+    (!(OBJ) || (uintptr_t) (OBJ) % __alignof__(OVS_TYPEOF(OBJ)) == 0)
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index c576ae6202a4..2e64fed5f8f2 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -3172,6 +3172,8 @@  mf_format(const struct mf_field *mf,
           const struct ofputil_port_map *port_map,
           struct ds *s)
 {
+    union mf_value aligned_mask;
+
     if (mask) {
         if (is_all_zeros(mask, mf->n_bytes)) {
             ds_put_cstr(s, "ANY");
@@ -3207,6 +3209,10 @@  mf_format(const struct mf_field *mf,
         break;
 
     case MFS_ETHERNET:
+        if (!IS_PTR_ALIGNED(mask)) {
+            memcpy(&aligned_mask.mac, mask, sizeof aligned_mask.mac);
+            mask = &aligned_mask;
+        }
         eth_format_masked(value->mac, mask ? &mask->mac : NULL, s);
         break;
 
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 440f5f7630c9..6e87deffc0d0 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -31,6 +31,7 @@ 
 #include "openvswitch/ofp-match.h"
 #include "openvswitch/ofp-port.h"
 #include "openvswitch/ofpbuf.h"
+#include "openvswitch/util.h"
 #include "openvswitch/vlog.h"
 #include "packets.h"
 #include "openvswitch/shash.h"
@@ -1491,9 +1492,15 @@  nx_put_entry(struct ofpbuf *b, const struct mf_field *mff,
              enum ofp_version version, const union mf_value *value,
              const union mf_value *mask)
 {
+    union mf_value aligned_mask;
     bool masked;
     int len, offset;
 
+    if (!IS_PTR_ALIGNED(mask)) {
+        memcpy(&aligned_mask, mask, mff->n_bytes);
+        mask = &aligned_mask;
+    }
+
     len = mf_field_len(mff, value, mask, &masked);
     offset = mff->n_bytes - len;
 
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index d2b26f8ee27d..c5f339c86eac 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -7119,10 +7119,15 @@  do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
 
             /* Set the field only if the packet actually has it. */
             if (mf_are_prereqs_ok(mf, flow, wc)) {
-                mf_mask_field_masked(mf, ofpact_set_field_mask(set_field), wc);
-                mf_set_flow_value_masked(mf, set_field->value,
-                                         ofpact_set_field_mask(set_field),
-                                         flow);
+                union mf_value *mask = ofpact_set_field_mask(set_field);
+                union mf_value aligned_mask;
+
+                if (!IS_PTR_ALIGNED(mask)) {
+                    memcpy(&aligned_mask, mask, mf->n_bytes);
+                    mask = &aligned_mask;
+                }
+                mf_mask_field_masked(mf, mask, wc);
+                mf_set_flow_value_masked(mf, set_field->value, mask, flow);
             } else {
                 xlate_report(ctx, OFT_WARN,
                              "unmet prerequisites for %s, set_field ignored",