On Wed, Oct 13, 2021 at 5:12 AM Aaron Conole <acon...@redhat.com> wrote: > > Ilya Maximets <i.maxim...@ovn.org> writes: > > > 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 803ed12e31b0 ("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. > > This is useful in case some future mechanism does the same. > > > 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. > > Thanks. > > > 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. > > :( > > There's a lot of work still needed for this feature. Separately, I've > noticed that the purge timeout isn't settable, and is much longer than > the 15s or so that is noted in the commit (on my system, it tested at > almost two minutes) >
Speaking of the purge timer issue, I think this is because the IPF module is packet-driven, in your test there must be no new frag packets received here. So ipf_send_frags_in_list can not be called. And eventually frags are purged by the ipf_clean thread. In ipf_clean thread, the purge timer is 1min exactly. > > Fixes: 7e6b41ac8d9d ("dpif-netdev: Fix crash when PACKET_OUT is metered.") > > Signed-off-by: Ilya Maximets <i.maxim...@ovn.org> > > --- > > Thanks for the quick fix. > > Acked-by: Aaron Conole <acon...@redhat.com> > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev