@@ -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'. */
@@ -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
@@ -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;
@@ -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;
@@ -7749,16 +7749,15 @@ 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 bad_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,
- ofperr_get_name(error), ds_cstr(&s));
+ VLOG_WARN("bad action at offset %"PRIuSIZE" (%s):\n%s",
+ bad_action_offset, ofperr_get_name(error), ds_cstr(&s));
ds_destroy(&s);
}
}
@@ -7769,25 +7768,46 @@ ofpacts_decode(const void *actions, size_t actions_len,
const struct vl_mff_map *vl_mff_map,
uint64_t *ofpacts_tlv_bitmap, struct ofpbuf *ofpacts)
{
- struct ofpbuf openflow = ofpbuf_const_initializer(actions, actions_len);
- while (openflow.size) {
- const struct ofp_action_header *action = openflow.data;
+ struct ofpbuf openflow_actions
+ = ofpbuf_const_initializer(actions, actions_len);
+ struct ofpbuf *openflow = &openflow_actions;
+
+ enum ofperr error = 0;
+ while (openflow->size) {
+ /* Ensure the next action data is properly aligned. Trimming removes
+ * all headroom and ensures alignment. The headroom is also removed
+ * when cloning and ofpbuf.
+ */
+ if (!OFPACT_IS_ALIGNED(openflow->data)) {
+ if (openflow == &openflow_actions) {
+ openflow = ofpbuf_clone(openflow);
+ } else {
+ ofpbuf_trim(openflow);
+ }
+ }
+
+ const struct ofp_action_header *action = openflow->data;
+ size_t action_offset = actions_len - openflow->size;
enum ofp_raw_action_type raw;
- enum ofperr error;
uint64_t arg;
- error = ofpact_pull_raw(&openflow, ofp_version, &raw, &arg);
+ error = ofpact_pull_raw(openflow, ofp_version, &raw, &arg);
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;
+ log_bad_action(actions, actions_len, action_offset, error);
+ goto done;
}
}
- return 0;
+
+done:
+ if (openflow != &openflow_actions) {
+ ofpbuf_delete(openflow);
+ }
+ return error;
}
static enum ofperr
@@ -20,6 +20,7 @@
#include "openflow/openflow.h"
#include "openflow/nicira-ext.h"
#include "openvswitch/dynamic-string.h"
+#include "openvswitch/ofp-actions.h"
#include "openvswitch/ofp-errors.h"
#include "openvswitch/ofp-msgs.h"
#include "openvswitch/ofp-print.h"
@@ -346,6 +347,7 @@ ofperr_decode_msg(const struct ofp_header *oh, struct ofpbuf *payload)
if (error && payload) {
ofpbuf_init(payload, b.size);
ofpbuf_push(payload, b.data, b.size);
+ ofpbuf_trim(payload);
}
return error;
}
@@ -7020,10 +7020,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",
When decoding multiple actions in ofpacts_decode(), make sure that when we advance to the next action it will be properly aligned (multiple of OFPACT_ALIGNTO). If not, clone or trim the ofpbuf to ensure proper aligment. One 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 Another example is when running one of the fuzz tests: lib/ofp-actions.c:5347:12: runtime error: member access within misaligned address 0x0000016ba274 for type 'const struct nx_action_learn', which requires 8 byte alignment 0x0000016ba274: note: pointer points here 20 20 20 20 ff ff 00 38 00 00 23 20 00 10 20 20 20 20 20 20 20 20 20 20 20 20 20 20 00 03 20 00 ^ #0 0x52cece in decode_LEARN_common lib/ofp-actions.c:5347 #1 0x52dcf6 in decode_NXAST_RAW_LEARN lib/ofp-actions.c:5463 #2 0x548604 in ofpact_decode lib/ofp-actions.inc2:4723 #3 0x53ee43 in ofpacts_decode lib/ofp-actions.c:7781 #4 0x53efc1 in ofpacts_pull_openflow_actions__ lib/ofp-actions.c:7820 #5 0x5409e1 in ofpacts_pull_openflow_instructions lib/ofp-actions.c:8396 #6 0x5608a8 in ofputil_decode_flow_stats_reply lib/ofp-flow.c:1100 [...] Or: lib/ofp-print.c:1218:24: runtime error: member access within misaligned address 0x0000019229d2 for type 'const struct ofp_header', which requires 4 byte alignment 0x0000019229d2: note: pointer points here 00 00 5a 5a 05 22 00 3e 00 00 00 09 00 00 00 00 00 00 00 03 05 0d 00 2e 00 00 00 09 ff ff ff ff ^ #0 0x7d45cc in ofp_to_string lib/ofp-print.c:1218 #1 0x774fa8 in ofperr_msg_format lib/ofp-errors.c:253 #2 0x7d2617 in ofp_print_error_msg lib/ofp-print.c:435 #3 0x7d3eb7 in ofp_to_string__ lib/ofp-print.c:998 #4 0x7d47fb in ofp_to_string lib/ofp-print.c:1244 #5 0x8dcc4b in do_send lib/vconn.c:688 #6 0x8dca64 in vconn_send lib/vconn.c:671 [...] Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- include/openvswitch/ofp-actions.h | 1 + include/openvswitch/util.h | 3 +++ lib/meta-flow.c | 6 +++++ lib/nx-match.c | 7 ++++++ lib/ofp-actions.c | 44 +++++++++++++++++++++++++++---------- lib/ofp-errors.c | 2 ++ ofproto/ofproto-dpif-xlate.c | 13 ++++++++--- 7 files changed, 60 insertions(+), 16 deletions(-)