netdev-windows: Add checking when creating netdev with system type on Windows
In the recent Antrea project testing, some port could not be created
on Windows.
When doing debug, our team found there is one case happening when multiple
ports are waiting for be created with correct port number.
Some system type port will be created netdev successfully and it will cause
conflict as in the dpif side it will be internal type. So finally the port
will be created failed and it could not be easily recovered.
With the patch, on Windows the netdev creating will be blocked for system
type when the ovs_tyep got on dpif is internal. More detailed case description
is in the reported issue No.262 with link below.
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.
Since 640d4db788ed ("ipf: Fix a use-after-free error, ...") the ipf
framework unconditionally allocates a new dp_packet to track
individual fragments. This prevents a use-after-free. However, an
additional issue was present - even when the packet buffer is cloned,
if the ip fragment handling code keeps it, the original buffer is
leaked during the refill loop. Even in the original processing code,
the hardcoded dnsteal branches would always leak a packet buffer from
the refill loop.
This can be confirmed with valgrind:
==717566== 16,672 (4,480 direct, 12,192 indirect) bytes in 8 blocks are definitely lost in loss record 390 of 390
==717566== at 0x484086F: malloc (vg_replace_malloc.c:380)
==717566== by 0x537BFD: xmalloc__ (util.c:137)
==717566== by 0x537BFD: xmalloc (util.c:172)
==717566== by 0x46DDD4: dp_packet_new (dp-packet.c:153)
==717566== by 0x46DDD4: dp_packet_new_with_headroom (dp-packet.c:163)
==717566== by 0x550AA6: netdev_linux_batch_rxq_recv_sock.constprop.0 (netdev-linux.c:1262)
==717566== by 0x5512AF: netdev_linux_rxq_recv (netdev-linux.c:1511)
==717566== by 0x4AB7E0: netdev_rxq_recv (netdev.c:727)
==717566== by 0x47F00D: dp_netdev_process_rxq_port (dpif-netdev.c:4699)
==717566== by 0x47FD13: dpif_netdev_run (dpif-netdev.c:5957)
==717566== by 0x4331D2: type_run (ofproto-dpif.c:370)
==717566== by 0x41DFD8: ofproto_type_run (ofproto.c:1768)
==717566== by 0x40A7FB: bridge_run__ (bridge.c:3245)
==717566== by 0x411269: bridge_run (bridge.c:3310)
==717566== by 0x406E6C: main (ovs-vswitchd.c:127)
The fix is to delete the original packet when it isn't able to be
reinserted into the packet batch. Subsequent valgrind runs show that
the packets are not leaked from the batch any longer.
Fixes: 640d4db788ed ("ipf: Fix a use-after-free error, and remove the 'do_not_steal' flag.")
Fixes: 4ea96698f667 ("Userspace datapath: Add fragmentation handling.")
Reported-by: Wan Junjie <email address hidden>
Reported-at: https://github.com/openvswitch/ovs-issues/issues/226
Signed-off-by: Aaron Conole <email address hidden>
Reviewed-by: David Marchand <email address hidden>
Tested-by: Wan Junjie <email address hidden>
Signed-off-by: Alin-Gabriel Serdean <email address hidden>
datapath-windows:adjust Offset when processing packet in POP_VLAN action
In one typical setup, on the Windows VM running OVS Windows Kernel, a Geneva
packet with 8021.q VLAN tag is received. Then it may do POP_VLAN action
processing in Actions.c, if the packet does not have Ieee8021QNetBufferListInfo
in the oob of the packet, it will be processed by function OvsPopVlanInPktBuf().
In the function it will go on remove VLAN header present in the nbl, but related
layers is never readjusted for the offset value at this moment. As a result, it
will cause function OvsValidateIPChecksum drop the packet.
cirrus: Reduce memory requirements for FreeBSD VMs.
According to memory usage graphs, our builds are using 3GB at most.
Reducing memory requirements to 4GB to have some room. This change
doesn't affect time needed to finish the build, but should have a
slight positive effect on scheduling time on a community cluster.
And it's also not cool from our side to reserve shared resources that
we're not using, while they could be used by some other project.
netdev-linux: Fix a null pointer dereference in netdev_linux_notify_sock().
If nl_sock_join_mcgroup() returns an error, the 'sock' is freed and
set to NULL. This issues will lead to null pointer deference in
nl_sock_listen_all_nsid(). To fix it, we call nl_sock_listen_all_nsid()
before joining the mcgroups.
Fixes: cf114a7fce80 ("netlink linux: enable listening to all nsids")
Signed-off-by: Yunjian Wang <email address hidden>
Acked-by: Flavio Leitner <email address hidden>
Reviewed-by: David Marchand <email address hidden>
Signed-off-by: Ilya Maximets <email address hidden>
