From a5e7296363137b4d77d1ae1ffb7bc2be5980bd9c Mon Sep 17 00:00:00 2001 From: qz_cx Date: Fri, 8 Jul 2022 13:59:53 +0800 Subject: [PATCH] ipf: release unhandled packets from the batch Since 640d4db ("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: 640d4db ("ipf: Fix a use-after-free error, and remove the 'do_not_steal' flag.") Fixes: 4ea9669 ("Userspace datapath: Add fragmentation handling.") Reported-by: Wan Junjie Reported-at: openvswitch/ovs-issues#226 Signed-off-by: Aaron Conole Reviewed-by: David Marchand Tested-by: Wan Junjie Signed-off-by: Alin-Gabriel Serdean --- lib/ipf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/ipf.c b/lib/ipf.c index 4cc0f2d..e8858d2 100644 --- a/lib/ipf.c +++ b/lib/ipf.c @@ -941,6 +941,8 @@ ipf_extract_frags_from_batch(struct ipf *ipf, struct dp_packet_batch *pb, if (!ipf_handle_frag(ipf, pkt, dl_type, zone, now, hash_basis, pb->do_not_steal)) { dp_packet_batch_refill(pb, pkt, pb_idx); + } else { + dp_packet_delete(pkt); } ovs_mutex_unlock(&ipf->ipf_lock); } else { -- 2.33.0