[ovs-dev,v6,5/7] ofp-actions: Use aligned structures when decoding ofp actions.

diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h
index 711e7c773d6c..7b57e49ad657 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -203,6 +203,7 @@  BUILD_ASSERT_DECL(sizeof(struct ofpact) == 4);
 /* Alignment. */
 #define OFPACT_ALIGNTO 8
 #define OFPACT_ALIGN(SIZE) ROUND_UP(SIZE, OFPACT_ALIGNTO)
+#define OFPACT_IS_ALIGNED(ADDR) ((uintptr_t) (ADDR) % OFPACT_ALIGNTO == 0)
 #define OFPACT_PADDED_MEMBERS(MEMBERS) PADDED_MEMBERS(OFPACT_ALIGNTO, MEMBERS)
 
 /* Returns the ofpact following 'ofpact'. */
diff --git a/include/openvswitch/ofpbuf.h b/include/openvswitch/ofpbuf.h
index 32f03ea8377c..1fc4a3a7f87e 100644
--- a/include/openvswitch/ofpbuf.h
+++ b/include/openvswitch/ofpbuf.h
@@ -111,6 +111,7 @@  void ofpbuf_use_ds(struct ofpbuf *, const struct ds *);
 void ofpbuf_use_stack(struct ofpbuf *, void *, size_t);
 void ofpbuf_use_stub(struct ofpbuf *, void *, size_t);
 void ofpbuf_use_const(struct ofpbuf *, const void *, size_t);
+void ofpbuf_use_data(struct ofpbuf *, const void *, size_t);
 
 void ofpbuf_init(struct ofpbuf *, size_t);
 void ofpbuf_uninit(struct ofpbuf *);
@@ -149,6 +150,7 @@  static inline size_t ofpbuf_msgsize(const struct ofpbuf *);
 void ofpbuf_prealloc_headroom(struct ofpbuf *, size_t);
 void ofpbuf_prealloc_tailroom(struct ofpbuf *, size_t);
 void ofpbuf_trim(struct ofpbuf *);
+void ofpbuf_align(struct ofpbuf *);
 void ofpbuf_padto(struct ofpbuf *, size_t);
 void ofpbuf_shift(struct ofpbuf *, int);
 
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index b098611833dc..d7e5f542a049 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -424,7 +424,8 @@  static void put_reg_load(struct ofpbuf *openflow,
                          const struct mf_subfield *, uint64_t value);
 
 static enum ofperr ofpact_pull_raw(struct ofpbuf *, enum ofp_version,
-                                   enum ofp_raw_action_type *, uint64_t *arg);
+                                   enum ofp_raw_action_type *, uint64_t *arg,
+                                   size_t *raw_len);
 static void *ofpact_put_raw(struct ofpbuf *, enum ofp_version,
                             enum ofp_raw_action_type, uint64_t arg);
 
@@ -7773,45 +7774,87 @@  check_GOTO_TABLE(const struct ofpact_goto_table *a,
 
 static void
 log_bad_action(const struct ofp_action_header *actions, size_t actions_len,
-               const struct ofp_action_header *bad_action, enum ofperr error)
+               size_t action_offset, enum ofperr error)
 {
     if (!VLOG_DROP_WARN(&rl)) {
         struct ds s;
 
         ds_init(&s);
         ds_put_hex_dump(&s, actions, actions_len, 0, false);
-        VLOG_WARN("bad action at offset %#"PRIxPTR" (%s):\n%s",
-                  (char *)bad_action - (char *)actions,
+        VLOG_WARN("bad action at offset %"PRIuSIZE" (%s):\n%s", action_offset,
                   ofperr_get_name(error), ds_cstr(&s));
         ds_destroy(&s);
     }
 }
 
 static enum ofperr
-ofpacts_decode(const void *actions, size_t actions_len,
-               enum ofp_version ofp_version,
-               const struct vl_mff_map *vl_mff_map,
-               uint64_t *ofpacts_tlv_bitmap, struct ofpbuf *ofpacts)
+ofpacts_decode_aligned(struct ofpbuf *openflow, enum ofp_version ofp_version,
+                       const struct vl_mff_map *vl_mff_map,
+                       uint64_t *ofpacts_tlv_bitmap, struct ofpbuf *ofpacts,
+                       size_t *bad_action_offset)
 {
-    struct ofpbuf openflow = ofpbuf_const_initializer(actions, actions_len);
-    while (openflow.size) {
-        const struct ofp_action_header *action = openflow.data;
+    size_t decoded_len = 0;
+    enum ofperr error = 0;
+
+    ovs_assert(OFPACT_IS_ALIGNED(openflow->data));
+    while (openflow->size) {
+        /* Ensure the next action data is properly aligned before decoding it.
+         * Some times it's valid to have to decode actions that are not
+         * properly aligned (e.g., when processing OF 1.0 statistics reply
+         * messages which have a header of 12 bytes - struct ofp10_stats_msg).
+         * In other cases the encoder might be buggy.
+         */
+        if (!OFPACT_IS_ALIGNED(openflow->data)) {
+            ofpbuf_align(openflow);
+        }
+
+        const struct ofp_action_header *action = openflow->data;
         enum ofp_raw_action_type raw;
-        enum ofperr error;
+        size_t act_len = 0;
         uint64_t arg;
 
-        error = ofpact_pull_raw(&openflow, ofp_version, &raw, &arg);
+        error = ofpact_pull_raw(openflow, ofp_version, &raw, &arg, &act_len);
         if (!error) {
             error = ofpact_decode(action, raw, ofp_version, arg, vl_mff_map,
                                   ofpacts_tlv_bitmap, ofpacts);
         }
 
         if (error) {
-            log_bad_action(actions, actions_len, action, error);
-            return error;
+            *bad_action_offset = decoded_len;
+            goto done;
         }
+        decoded_len += act_len;
     }
-    return 0;
+
+done:
+    return error;
+}
+
+static enum ofperr
+ofpacts_decode(const void *actions, size_t actions_len,
+               enum ofp_version ofp_version,
+               const struct vl_mff_map *vl_mff_map,
+               uint64_t *ofpacts_tlv_bitmap, struct ofpbuf *ofpacts)
+{
+    size_t bad_action_offset = 0;
+    struct ofpbuf aligned_buf;
+
+    if (!OFPACT_IS_ALIGNED(actions)) {
+        ofpbuf_init(&aligned_buf, actions_len);
+        ofpbuf_put(&aligned_buf, actions, actions_len);
+    } else {
+        ofpbuf_use_data(&aligned_buf, actions, actions_len);
+    }
+
+    enum ofperr error
+        = ofpacts_decode_aligned(&aligned_buf, ofp_version, vl_mff_map,
+                                 ofpacts_tlv_bitmap, ofpacts,
+                                 &bad_action_offset);
+    if (error) {
+        log_bad_action(actions, actions_len, bad_action_offset, error);
+    }
+    ofpbuf_uninit(&aligned_buf);
+    return error;
 }
 
 static enum ofperr
@@ -9664,14 +9707,15 @@  ofpact_decode_raw(enum ofp_version ofp_version,
 
 static enum ofperr
 ofpact_pull_raw(struct ofpbuf *buf, enum ofp_version ofp_version,
-                enum ofp_raw_action_type *raw, uint64_t *arg)
+                enum ofp_raw_action_type *raw, uint64_t *arg,
+                size_t *raw_len)
 {
     const struct ofp_action_header *oah = buf->data;
     const struct ofpact_raw_instance *action;
     unsigned int length;
     enum ofperr error;
 
-    *raw = *arg = 0;
+    *raw = *arg = *raw_len = 0;
     error = ofpact_decode_raw(ofp_version, oah, buf->size, &action);
     if (error) {
         return error;
@@ -9714,6 +9758,7 @@  ofpact_pull_raw(struct ofpbuf *buf, enum ofp_version ofp_version,
     }
 
     ofpbuf_pull(buf, length);
+    *raw_len = length;
 
     return 0;
 }
diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c
index 0330681b2518..79ced46d7d73 100644
--- a/lib/ofpbuf.c
+++ b/lib/ofpbuf.c
@@ -115,6 +115,24 @@  ofpbuf_use_const(struct ofpbuf *b, const void *data, size_t size)
     ofpbuf_use__(b, CONST_CAST(void *, data), size, size, OFPBUF_STACK);
 }
 
+/* Initializes 'b' as an ofpbuf whose data starts at 'data' and continues for
+ * 'size' bytes.  This is appropriate for an ofpbuf that will be mostly used to
+ * inspect existing data, however, if needed, data may be occasionally changed.
+ *
+ * The first time the data is changed the provided buffer will be copied into
+ * a malloc()'d buffer.  Thus, it is wise to call ofpbuf_uninit() on an ofpbuf
+ * initialized by this function, so that if it expanded into the heap, that
+ * memory is freed.
+ *
+ * 'base' should be appropriately aligned.  Using an array of uint32_t or
+ * uint64_t for the buffer is a reasonable way to ensure appropriate alignment
+ * for 32- or 64-bit data. */
+void
+ofpbuf_use_data(struct ofpbuf *b, const void *data, size_t size)
+{
+    ofpbuf_use__(b, CONST_CAST(void *, data), size, size, OFPBUF_STUB);
+}
+
 /* Initializes 'b' as an empty ofpbuf with an initial capacity of 'size'
  * bytes. */
 void
@@ -322,6 +340,27 @@  ofpbuf_trim(struct ofpbuf *b)
     }
 }
 
+/* Re-aligns the buffer data.  Relies on malloc() to ensure proper alignment.
+ *
+ * This function should not be called for buffers of type OFPBUF_STACK.
+ */
+void
+ofpbuf_align(struct ofpbuf *b)
+{
+    switch (b->source) {
+    case OFPBUF_MALLOC:
+    case OFPBUF_STUB:
+        /* Resizing 'b' always reallocates the buffer, ensuring proper
+         * alignment.
+         */
+        ofpbuf_resize__(b, 0, 0);
+        break;
+    case OFPBUF_STACK:
+        OVS_NOT_REACHED();
+        break;
+    }
+}
+
 /* If 'b' is shorter than 'length' bytes, pads its tail out with zeros to that
  * length. */
 void