summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAaron Conole <aconole@redhat.com>2021-10-05 14:18:44 -0400
committerAlin-Gabriel Serdean <aserdean@ovn.org>2021-10-12 18:43:46 +0300
commit9efa2ea619e563f7ec70c93e4eefb4b96cdfd7d4 (patch)
treee5829aa43ba215128e7ab729c769e87f326bd1e8
parentf8274b78c3403591e84f3c2bbacf8c86920d68ba (diff)
downloadopenvswitch-9efa2ea619e563f7ec70c93e4eefb4b96cdfd7d4.tar.gz
ipf: release unhandled packets from the batch
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 <wanjunjie@bytedance.com> Reported-at: https://github.com/openvswitch/ovs-issues/issues/226 Signed-off-by: Aaron Conole <aconole@redhat.com> Reviewed-by: David Marchand <david.marchand@redhat.com> Tested-by: Wan Junjie <wanjunjie@bytedance.com> Signed-off-by: Alin-Gabriel Serdean <aserdean@ovn.org>
-rw-r--r--lib/ipf.c2
1 files changed, 2 insertions, 0 deletions
diff --git a/lib/ipf.c b/lib/ipf.c
index 24325a638..009f5d1e9 100644
--- a/lib/ipf.c
+++ b/lib/ipf.c
@@ -940,6 +940,8 @@ ipf_extract_frags_from_batch(struct ipf *ipf, struct dp_packet_batch *pb,
ovs_mutex_lock(&ipf->ipf_lock);
if (!ipf_handle_frag(ipf, pkt, dl_type, zone, now, hash_basis)) {
dp_packet_batch_refill(pb, pkt, pb_idx);
+ } else {
+ dp_packet_delete(pkt);
}
ovs_mutex_unlock(&ipf->ipf_lock);
} else {