diff options
author | Ilya Maximets <i.maximets@ovn.org> | 2020-10-14 18:13:46 +0200 |
---|---|---|
committer | Ilya Maximets <i.maximets@ovn.org> | 2020-10-15 13:37:24 +0200 |
commit | 6eae20e88fdd8dc786ead8fc03d8fd3cc0511724 (patch) | |
tree | bdb1b6fde638f73835d44459628e618256d246fe | |
parent | 1858e7064fd823bf3fc43776c0787903d9dcbc6a (diff) | |
download | openvswitch-6eae20e88fdd8dc786ead8fc03d8fd3cc0511724.tar.gz |
ofp-ed-props: Fix using uninitialized padding for NSH encap actions.
OVS uses memcmp to compare actions of existing and new flows, but
'struct ofp_ed_prop_nsh_md_type' and corresponding ofpact structure has
3 bytes of padding that never initialized and passed around within OF
data structures and messages.
Uninitialized bytes in MemcmpInterceptorCommon
at offset 21 inside [0x7090000003f8, 136)
WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x4a184e in bcmp (vswitchd/ovs-vswitchd+0x4a184e)
#1 0x896c8a in ofpacts_equal lib/ofp-actions.c:9121:31
#2 0x564403 in replace_rule_finish ofproto/ofproto.c:5650:37
#3 0x563462 in add_flow_finish ofproto/ofproto.c:5218:13
#4 0x54a1ff in ofproto_flow_mod_finish ofproto/ofproto.c:8091:17
#5 0x5433b2 in handle_flow_mod__ ofproto/ofproto.c:6216:17
#6 0x56a2fc in handle_flow_mod ofproto/ofproto.c:6190:17
#7 0x565bda in handle_single_part_openflow ofproto/ofproto.c:8504:16
#8 0x540b25 in handle_openflow ofproto/ofproto.c:8685:21
#9 0x6697fd in ofconn_run ofproto/connmgr.c:1329:13
#10 0x668e6e in connmgr_run ofproto/connmgr.c:356:9
#11 0x53f1bc in ofproto_run ofproto/ofproto.c:1890:5
#12 0x4ead0c in bridge_run__ vswitchd/bridge.c:3250:9
#13 0x4e9bc8 in bridge_run vswitchd/bridge.c:3309:5
#14 0x51c072 in main vswitchd/ovs-vswitchd.c:127:9
#15 0x7f23a99011a2 in __libc_start_main (/lib64/libc.so.6)
#16 0x46b92d in _start (vswitchd/ovs-vswitchd+0x46b92d)
Uninitialized value was stored to memory at
#0 0x4745aa in __msan_memcpy.part.0 (vswitchd/ovs-vswitchd)
#1 0x54529f in rule_actions_create ofproto/ofproto.c:3134:5
#2 0x54915e in ofproto_rule_create ofproto/ofproto.c:5284:11
#3 0x55d419 in add_flow_init ofproto/ofproto.c:5123:17
#4 0x54841f in ofproto_flow_mod_init ofproto/ofproto.c:7987:17
#5 0x543250 in handle_flow_mod__ ofproto/ofproto.c:6206:13
#6 0x56a2fc in handle_flow_mod ofproto/ofproto.c:6190:17
#7 0x565bda in handle_single_part_openflow ofproto/ofproto.c:8504:16
#8 0x540b25 in handle_openflow ofproto/ofproto.c:8685:21
#9 0x6697fd in ofconn_run ofproto/connmgr.c:1329:13
#10 0x668e6e in connmgr_run ofproto/connmgr.c:356:9
#11 0x53f1bc in ofproto_run ofproto/ofproto.c:1890:5
#12 0x4ead0c in bridge_run__ vswitchd/bridge.c:3250:9
#13 0x4e9bc8 in bridge_run vswitchd/bridge.c:3309:5
#14 0x51c072 in main vswitchd/ovs-vswitchd.c:127:9
#15 0x7f23a99011a2 in __libc_start_main (/lib64/libc.so.6)
Uninitialized value was created by an allocation of 'ofpacts_stub'
in the stack frame of function 'handle_flow_mod'
#0 0x569e80 in handle_flow_mod ofproto/ofproto.c:6170
This could cause issues with flow modifications or other operations.
To reproduce, some NSH tests could be run under valgrind or clang
MemorySantizer. Ex. "nsh - md1 encap over a veth link" test.
Fix that by clearing padding bytes while encoding and decoding.
OVS will still accept OF messages with non-zero padding from
controllers.
New tests added to tests/ofp-actions.at.
Fixes: 1fc11c5948cf ("Generic encap and decap support for NSH")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Jan Scheurich <jan.scheurich@ericsson.com>
-rw-r--r-- | lib/ofp-ed-props.c | 3 | ||||
-rw-r--r-- | tests/ofp-actions.at | 11 |
2 files changed, 13 insertions, 1 deletions
diff --git a/lib/ofp-ed-props.c b/lib/ofp-ed-props.c index 901da2f0d..08c69286a 100644 --- a/lib/ofp-ed-props.c +++ b/lib/ofp-ed-props.c @@ -49,7 +49,7 @@ decode_ed_prop(const struct ofp_ed_prop_header **ofp_prop, return OFPERR_NXBAC_BAD_ED_PROP; } struct ofpact_ed_prop_nsh_md_type *pnmt = - ofpbuf_put_uninit(out, sizeof(*pnmt)); + ofpbuf_put_zeros(out, sizeof *pnmt); pnmt->header.prop_class = prop_class; pnmt->header.type = prop_type; pnmt->header.len = len; @@ -108,6 +108,7 @@ encode_ed_prop(const struct ofpact_ed_prop **prop, opnmt->header.len = offsetof(struct ofp_ed_prop_nsh_md_type, pad); opnmt->md_type = pnmt->md_type; + memset(opnmt->pad, 0, sizeof opnmt->pad); prop_len = sizeof(*pnmt); break; } diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at index e320a92a8..f47212259 100644 --- a/tests/ofp-actions.at +++ b/tests/ofp-actions.at @@ -746,6 +746,17 @@ dnl Check the ONF extension form of "copy_field". # actions=move:NXM_OF_IN_PORT[]->NXM_OF_VLAN_TCI[] ffff 0020 4f4e4600 0c80 0000 0010 0000 0000 0000 00000002 00000802 00000000 +dnl Check NSH encap (experimenter extension). +# actions=encap(nsh(md_type=1)) +ffff 0018 00002320 002e 0000 0001894f 0004 01 05 01 000000 + +dnl NSH encap with non-zero padding. +# actions=encap(nsh(md_type=1)) +# 21: 12 -> 00 +# 22: 34 -> 00 +# 23: 56 -> 00 +ffff 0018 00002320 002e 0000 0001894f 0004 01 05 01 123456 + ]) sed '/^[[#&]]/d' < test-data > input.txt sed -n 's/^# //p; /^$/p' < test-data > expout |