[ovs-git] [openvswitch/ovs] 257d62: tunnel-push-pop.at: Mask source port in tunnel hea...

Ilya Maximets noreply at github.com
Wed Oct 13 13:55:13 UTC 2021


  Branch: refs/heads/branch-2.14
  Home:   https://github.com/openvswitch/ovs
  Commit: 257d62a90a90bd9b79a3b0769cc92a30eff81c55
      https://github.com/openvswitch/ovs/commit/257d62a90a90bd9b79a3b0769cc92a30eff81c55
  Author: Ilya Maximets <i.maximets at ovn.org>
  Date:   2021-10-12 (Tue, 12 Oct 2021)

  Changed paths:
    M tests/tunnel-push-pop.at

  Log Message:
  -----------
  tunnel-push-pop.at: Mask source port in tunnel header.

Source port is based on a packet hash and hash depends on a chosen
implementation.  Masking it to avoid test failures with '-msse4.2'.

Fixes: 7e6b41ac8d9d ("dpif-netdev: Fix crash when PACKET_OUT is metered.")
Reported-by: Kumar Amber <kumar.amber at intel.com>
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
Acked-by: Alin-Gabriel Serdean <aserdean at ovn.org>


  Commit: 5861958e36632bfa22866a2dd9d6d294ad7e262a
      https://github.com/openvswitch/ovs/commit/5861958e36632bfa22866a2dd9d6d294ad7e262a
  Author: Ilya Maximets <i.maximets at ovn.org>
  Date:   2021-10-13 (Wed, 13 Oct 2021)

  Changed paths:
    M lib/dpif-netdev.c

  Log Message:
  -----------
  dpif-netdev: Fix use-after-free on PACKET_OUT of IP fragments.

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.

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: 7e6b41ac8d9d ("dpif-netdev: Fix crash when PACKET_OUT is metered.")
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
Acked-by: Aaron Conole <aconole at redhat.com>


Compare: https://github.com/openvswitch/ovs/compare/b5e3fd038dfd...5861958e3663


More information about the git mailing list