forked from openvswitch/ovs
-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix VS2019 16.10 build error #6
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fw2568
pushed a commit
that referenced
this pull request
Sep 7, 2022
IP fragmentation engine may not only steal the packet but also add more. For example, after receiving the last fragment, it will add all previous fragments to a batch. Unfortunately, it will also free the original last fragment and replace it with a copy. This invalidates the 'packet_clone' pointer in the dpif_netdev_execute() leading to the use-after-free: ==3525086==ERROR: AddressSanitizer: heap-use-after-free on address 0x61600020439c at pc 0x000000688a6d READ of size 1 at 0x61600020439c thread T0 #0 0x688a6c in dp_packet_swap ./lib/dp-packet.h:265:5 #1 0x68781d in dpif_netdev_execute lib/dpif-netdev.c:4103:9 #2 0x6675db in dpif_netdev_operate lib/dpif-netdev.c:4129:25 #3 0x691e5e in dpif_operate lib/dpif.c:1367:13 #4 0x692909 in dpif_execute lib/dpif.c:1321:9 #5 0x5b19c6 in packet_execute ofproto/ofproto-dpif.c:4991:5 #6 0x5a2861 in ofproto_packet_out_finish ofproto/ofproto.c:3662:5 #7 0x5a65c6 in do_bundle_commit ofproto/ofproto.c:8270:13 #8 0x5a0cae in handle_bundle_control ofproto/ofproto.c:8309:17 #9 0x59a476 in handle_single_part_openflow ofproto/ofproto.c:8593:16 #10 0x5877ac in handle_openflow ofproto/ofproto.c:8674:21 #11 0x6296f1 in ofconn_run ofproto/connmgr.c:1329:13 #12 0x62925d in connmgr_run ofproto/connmgr.c:356:9 #13 0x586904 in ofproto_run ofproto/ofproto.c:1879:5 #14 0x55c830 in bridge_run__ vswitchd/bridge.c:3251:9 #15 0x55c015 in bridge_run vswitchd/bridge.c:3310:5 #16 0x575f31 in main vswitchd/ovs-vswitchd.c:127:9 #17 0x7f01099d3492 in __libc_start_main (/lib64/libc.so.6+0x23492) #18 0x47d96d in _start (vswitchd/ovs-vswitchd+0x47d96d) 0x61600020439c is located 28 bytes inside of 560-byte region freed by thread T0 here: #0 0x5177a8 in free (vswitchd/ovs-vswitchd+0x5177a8) #1 0x6b17b6 in dp_packet_delete ./lib/dp-packet.h:256:9 #2 0x6afeee in ipf_extract_frags_from_batch lib/ipf.c:947:17 #3 0x6afd63 in ipf_preprocess_conntrack lib/ipf.c:1232:9 #4 0x946b2c in conntrack_execute lib/conntrack.c:1446:5 #5 0x67e3ed in dp_execute_cb lib/dpif-netdev.c:8277:9 #6 0x7097d7 in odp_execute_actions lib/odp-execute.c:865:17 #7 0x66409e in dp_netdev_execute_actions lib/dpif-netdev.c:8322:5 #8 0x6877ad in dpif_netdev_execute lib/dpif-netdev.c:4090:5 #9 0x6675db in dpif_netdev_operate lib/dpif-netdev.c:4129:25 #10 0x691e5e in dpif_operate lib/dpif.c:1367:13 #11 0x692909 in dpif_execute lib/dpif.c:1321:9 #12 0x5b19c6 in packet_execute ofproto/ofproto-dpif.c:4991:5 #13 0x5a2861 in ofproto_packet_out_finish ofproto/ofproto.c:3662:5 #14 0x5a65c6 in do_bundle_commit ofproto/ofproto.c:8270:13 #15 0x5a0cae in handle_bundle_control ofproto/ofproto.c:8309:17 #16 0x59a476 in handle_single_part_openflow ofproto/ofproto.c:8593:16 #17 0x5877ac in handle_openflow ofproto/ofproto.c:8674:21 #18 0x6296f1 in ofconn_run ofproto/connmgr.c:1329:13 #19 0x62925d in connmgr_run ofproto/connmgr.c:356:9 #20 0x586904 in ofproto_run ofproto/ofproto.c:1879:5 #21 0x55c830 in bridge_run__ vswitchd/bridge.c:3251:9 #22 0x55c015 in bridge_run vswitchd/bridge.c:3310:5 #23 0x575f31 in main vswitchd/ovs-vswitchd.c:127:9 #24 0x7f01099d3492 in __libc_start_main (/lib64/libc.so.6+0x23492) The issue can be reproduced with system-userspace testsuite on the 'conntrack - IPv4 fragmentation with fragments specified' test. Previously, there was a leak inside the IP fragmentation module that kept the original segment, so 'packet_clone' remained a valid pointer. But commit 803ed12 ("ipf: release unhandled packets from the batch") fixed the leak leading to use-after-free. Using the packet from a batch instead of 'packet_clone' to swap packet content to avoid the issue. While investigating this problem, more issues uncovered. One of them is that IP fragmentation engine can add more packets to the batch, but there is no way to get them to a caller. Adding an extra branch for this case with a 'FIXME' comment in order to highlight the issue. Another one is that IP fragmentation engine will keep only 32 fragments dropping all other fragments while refilling a batch, but that should be fixed separately. Fixes: 7e6b41a ("dpif-netdev: Fix crash when PACKET_OUT is metered.") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Aaron Conole <aconole@redhat.com>
fw2568
pushed a commit
that referenced
this pull request
Sep 7, 2022
This commit improves handling of packets where the allocated memory is less than 64 bytes. For packets recevied from DPDK ports this never matters, as an mbuf always pre-allocates enough space, however this can occur in cases where packet received from a kernel interface or injected by an OpenFlow controller. The fix is required to ensure OVS doesn't overread the allocated memory, e.g.: ==49944==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6060000d8181 at pc 0x000001cb9d24 bp 0x7ffce3b385d0 sp 0x7ffce3b385c8 READ of size 64 at 0x6060000d8181 thread T0 #0 0x1cb9d23 in mfex_avx512_process lib/dpif-netdev-extract-avx512.c:491:26 #1 0x1cb9d23 in mfex_avx512_ip_udp lib/dpif-netdev-extract-avx512.c:625:1 #2 0x18786a1 in dpif_miniflow_extract_autovalidator lib/dpif-netdev-private-extract.c:277:29 #3 0x1cbca5c in dp_netdev_input_outer_avx512 lib/dpif-netdev-avx512.c:159:19 #4 0x1853048 in dp_netdev_process_rxq_port lib/dpif-netdev.c:4900:19 #5 0x1837c76 in dpif_netdev_run lib/dpif-netdev.c:6197:25 #6 0x1727a02 in type_run ofproto/ofproto-dpif.c:370:9 #7 0x16f6e07 in ofproto_type_run ofproto/ofproto.c:1778:31 #8 0x16c1a8b in bridge_run__ vswitchd/bridge.c:3245:9 #9 0x16bd2fd in bridge_run vswitchd/bridge.c:3310:5 #10 0x16db8fe in main vswitchd/ovs-vswitchd.c:127:9 #11 0x7fbc0c5b61a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) #12 0xedabbd in _start (vswitchd/ovs-vswitchd+0xedabbd) 0x6060000d8181 is located 9 bytes to the right of 56-byte region [0x6060000d8140,0x6060000d8178) allocated by thread T0 here: #0 0xf7b09f in malloc (vswitchd/ovs-vswitchd+0xf7b09f) #1 0x1aff3b9 in xmalloc__ lib/util.c:137:15 #2 0x1aff3b9 in xmalloc lib/util.c:172:12 #3 0x1afe211 in process_command lib/unixctl.c:310:13 #4 0x1afe211 in run_connection lib/unixctl.c:344:17 #5 0x1afe211 in unixctl_server_run lib/unixctl.c:395:21 #6 0x16db918 in main vswitchd/ovs-vswitchd.c:128:9 #7 0x7fbc0c5b61a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) The solution implemented uses a mask-to-zero if the available buffer size is less than 64 bytes, and a branch for which type of load is used. Fixes: 250cedd ("dpif-netdev/mfex: Add AVX512 based optimized miniflow extract") Reported-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
fw2568
pushed a commit
that referenced
this pull request
Sep 7, 2022
Found by AddressSanitizer when running OVN tests: Direct leak of 64 byte(s) in 1 object(s) allocated from: #0 0x498fb2 in calloc (/ic/ovn-ic+0x498fb2) #1 0x5f681e in xcalloc__ ovs/lib/util.c:121:31 #2 0x5f681e in xzalloc__ ovs/lib/util.c:131:12 #3 0x5f681e in xzalloc ovs/lib/util.c:165:12 #4 0x5e3697 in ovsdb_idl_txn_add_map_op ovs/lib/ovsdb-idl.c:4057:29 #5 0x4d3f25 in update_isb_pb_external_ids ic/ovn-ic.c:576:5 #6 0x4cc4cc in create_isb_pb ic/ovn-ic.c:716:5 #7 0x4cc4cc in port_binding_run ic/ovn-ic.c:803:21 #8 0x4cc4cc in ovn_db_run ic/ovn-ic.c:1700:5 #9 0x4c9c1c in main ic/ovn-ic.c:1984:17 #10 0x7f9ad9f4a0b2 in __libc_start_main Signed-off-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
fw2568
pushed a commit
that referenced
this pull request
Sep 7, 2022
UB Sanitizer report: lib/stopwatch.c:119:22: runtime error: index 18446744073709551615 out of bounds for type 'long long unsigned int [50]' #0 0x698358 in calc_percentile lib/stopwatch.c:119 #1 0x69ada1 in add_sample lib/stopwatch.c:231 #2 0x69c086 in stopwatch_end_sample_protected lib/stopwatch.c:386 #3 0x69c522 in stopwatch_thread lib/stopwatch.c:441 #4 0x684bae in ovsthread_wrapper lib/ovs-thread.c:383 #5 0x7f042838b298 in start_thread (/lib64/libpthread.so.0+0x9298) #6 0x7f04277f2352 in clone (/lib64/libc.so.6+0x100352) Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Dumitru Ceara <dceara@redhat.com> Acked-by: Paolo Valerio <pvalerio@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
fw2568
pushed a commit
that referenced
this pull request
Sep 7, 2022
Reported by UndefinedBehaviorSanitizer: tests/idltest.c:3602:12: runtime error: member access within null pointer of type 'const struct idltest_simple' #0 0x4295af in idltest_simple_cursor_first_ge tests/idltest.c:3602 #1 0x41c81b in test_idl_compound_index_single_column tests/test-ovsdb.c:3128 #2 0x41e035 in do_idl_compound_index tests/test-ovsdb.c:3277 #3 0x4cf640 in ovs_cmdl_run_command__ lib/command-line.c:247 #4 0x4cf79f in ovs_cmdl_run_command lib/command-line.c:278 #5 0x4072f7 in main tests/test-ovsdb.c:79 #6 0x7fa858675b74 in __libc_start_main (/lib64/libc.so.6+0x27b74) #7 0x4060ed in _start (/root/ovs/tests/test-ovsdb+0x4060ed) Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Dumitru Ceara <dceara@redhat.com> Acked-by: Paolo Valerio <pvalerio@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
fw2568
pushed a commit
that referenced
this pull request
Sep 7, 2022
When compiled with clang and '-fsanitize=undefined' set, running 'ovsdb-client --timestamp monitor Open_vSwitch' in a sandbox triggers the following undefined behavior (flagged by UBSan): lib/dynamic-string.c:207:38: runtime error: applying zero offset to null pointer #0 0x4ebc18 in ds_put_strftime_msec lib/dynamic-string.c:207:38 #1 0x4ebd04 in xastrftime_msec lib/dynamic-string.c:225:5 #2 0x552e6a in table_format_timestamp__ lib/table.c:226:12 #3 0x552852 in table_print_timestamp__ lib/table.c:233:27 #4 0x5506f3 in table_print_table__ lib/table.c:254:5 #5 0x550633 in table_format lib/table.c:601:9 #6 0x5524f3 in table_print lib/table.c:633:5 #7 0x44dc5e in monitor_print_table ovsdb/ovsdb-client.c:1019:5 #8 0x44c650 in monitor_print ovsdb/ovsdb-client.c:1040:13 #9 0x44ac56 in do_monitor__ ovsdb/ovsdb-client.c:1500:21 #10 0x44636e in do_monitor ovsdb/ovsdb-client.c:1575:5 #11 0x442c41 in main ovsdb/ovsdb-client.c:283:5 Reported-by: Ilya Maximets <i.maximets@ovn.org> Signed-off-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
fw2568
pushed a commit
that referenced
this pull request
Sep 7, 2022
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior in lib/dpif-netlink.c:1077:40: runtime error: left shift of 1 by 31 places cannot be represented in type 'int' #0 0x73fc31 in dpif_netlink_port_add_compat lib/dpif-netlink.c:1077:40 #1 0x73fc31 in dpif_netlink_port_add lib/dpif-netlink.c:1132:17 #2 0x2c1745 in dpif_port_add lib/dpif.c:597:13 #3 0x07b279 in port_add ofproto/ofproto-dpif.c:3957:17 #4 0x01b209 in ofproto_port_add ofproto/ofproto.c:2124:13 #5 0xfdbfce in iface_do_create vswitchd/bridge.c:2066:13 #6 0xfdbfce in iface_create vswitchd/bridge.c:2109:13 #7 0xfdbfce in bridge_add_ports__ vswitchd/bridge.c:1173:21 #8 0xfb5319 in bridge_add_ports vswitchd/bridge.c:1189:5 #9 0xfb5319 in bridge_reconfigure vswitchd/bridge.c:901:9 #10 0xfae0f9 in bridge_run vswitchd/bridge.c:3334:9 #11 0xfe67dd in main vswitchd/ovs-vswitchd.c:129:9 #12 0x4b6d8f (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) #13 0x4b6e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) #14 0x562594eed024 in _start (vswitchd/ovs-vswitchd+0x787024) Fixes: 526df7d ("tunnel: Provide framework for tunnel extensions for VXLAN-GBP and others") Acked-by: Mike Pattrick <mkp@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
fw2568
pushed a commit
that referenced
this pull request
Nov 1, 2022
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior in lib/dpif-netlink.c:1077:40: runtime error: left shift of 1 by 31 places cannot be represented in type 'int' #0 0x73fc31 in dpif_netlink_port_add_compat lib/dpif-netlink.c:1077:40 #1 0x73fc31 in dpif_netlink_port_add lib/dpif-netlink.c:1132:17 #2 0x2c1745 in dpif_port_add lib/dpif.c:597:13 #3 0x07b279 in port_add ofproto/ofproto-dpif.c:3957:17 #4 0x01b209 in ofproto_port_add ofproto/ofproto.c:2124:13 #5 0xfdbfce in iface_do_create vswitchd/bridge.c:2066:13 #6 0xfdbfce in iface_create vswitchd/bridge.c:2109:13 #7 0xfdbfce in bridge_add_ports__ vswitchd/bridge.c:1173:21 #8 0xfb5319 in bridge_add_ports vswitchd/bridge.c:1189:5 #9 0xfb5319 in bridge_reconfigure vswitchd/bridge.c:901:9 #10 0xfae0f9 in bridge_run vswitchd/bridge.c:3334:9 #11 0xfe67dd in main vswitchd/ovs-vswitchd.c:129:9 #12 0x4b6d8f (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) #13 0x4b6e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) #14 0x562594eed024 in _start (vswitchd/ovs-vswitchd+0x787024) Fixes: 526df7d ("tunnel: Provide framework for tunnel extensions for VXLAN-GBP and others") Acked-by: Mike Pattrick <mkp@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.