From 933aaf9444a6e109d360425a98aac6cd0891ef60 Mon Sep 17 00:00:00 2001 From: Dumitru Ceara Date: Mon, 11 Apr 2022 13:38:13 +0200 Subject: ofp-actions: Ensure aligned accesses to masked fields. 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 To avoid this we now change the internal representation of the set_field actions, struct ofpact_set_field, such that the mask is always stored at a correctly aligned address, multiple of OFPACT_ALIGNTO. We also need to adapt the "ovs-ofctl show-flows - Oversized flow" test because now the ofpact representation of the set_field action uses more bytes in memory (for the extra alignment). Change the test to use dec_ttl instead. Acked-by: Aaron Conole Signed-off-by: Dumitru Ceara Signed-off-by: Ilya Maximets --- include/openvswitch/ofp-actions.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h index b7231c7bb..711e7c773 100644 --- a/include/openvswitch/ofp-actions.h +++ b/include/openvswitch/ofp-actions.h @@ -549,7 +549,8 @@ struct ofpact_set_field { const struct mf_field *field; ); union mf_value value[]; /* Significant value bytes followed by - * significant mask bytes. */ + * significant mask bytes aligned at + * OFPACT_ALIGNTO bytes. */ }; BUILD_ASSERT_DECL(offsetof(struct ofpact_set_field, value) % OFPACT_ALIGNTO == 0); @@ -557,9 +558,10 @@ BUILD_ASSERT_DECL(offsetof(struct ofpact_set_field, value) == sizeof(struct ofpact_set_field)); /* Use macro to not have to deal with constness. */ -#define ofpact_set_field_mask(SF) \ - ALIGNED_CAST(union mf_value *, \ - (uint8_t *)(SF)->value + (SF)->field->n_bytes) +#define ofpact_set_field_mask(SF) \ + ALIGNED_CAST(union mf_value *, \ + (uint8_t *)(SF)->value + \ + ROUND_UP((SF)->field->n_bytes, OFPACT_ALIGNTO)) /* OFPACT_PUSH_VLAN/MPLS/PBB * -- cgit v1.2.1