64 lines
2.9 KiB
Diff
64 lines
2.9 KiB
Diff
|
From a5e7296363137b4d77d1ae1ffb7bc2be5980bd9c Mon Sep 17 00:00:00 2001
|
||
|
|
From: qz_cx <wangqingzheng@kylinos.cn>
|
||
|
|
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 <wanjunjie@bytedance.com>
|
||
|
|
Reported-at: openvswitch/ovs-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>
|
||
|
|
---
|
||
|
|
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
|
||
|
|
|