From 28d18b673ffa2d13112ddb6e4c32c60d9b0cda50 Mon Sep 17 00:00:00 2001 From: Daniel Borkmann Date: Fri, 25 Aug 2023 15:49:45 +0200 Subject: net: Fix skb consume leak in sch_handle_egress Fix a memory leak for the tc egress path with TC_ACT_{STOLEN,QUEUED,TRAP}: [...] unreferenced object 0xffff88818bcb4f00 (size 232): comm "softirq", pid 0, jiffies 4299085078 (age 134.028s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 80 70 61 81 88 ff ff 00 41 31 14 81 88 ff ff ..pa.....A1..... backtrace: [] kmem_cache_alloc_node+0x268/0x400 [] __alloc_skb+0x211/0x2c0 [] alloc_skb_with_frags+0xbe/0x6b0 [] sock_alloc_send_pskb+0x6a9/0x870 [] __ip_append_data+0x14d0/0x3bf0 [] ip_append_data+0xee/0x190 [] icmp_push_reply+0xa6/0x470 [] icmp_reply+0x900/0xa00 [] icmp_echo.part.0+0x1a3/0x230 [] icmp_echo+0xcd/0x190 [] icmp_rcv+0x806/0xe10 [] ip_protocol_deliver_rcu+0x351/0x3d0 [] ip_local_deliver_finish+0x2b4/0x450 [] ip_local_deliver+0x174/0x1f0 [] ip_sublist_rcv_finish+0x1f2/0x420 [] ip_sublist_rcv+0x466/0x920 [...] I was able to reproduce this via: ip link add dev dummy0 type dummy ip link set dev dummy0 up tc qdisc add dev eth0 clsact tc filter add dev eth0 egress protocol ip prio 1 u32 match ip protocol 1 0xff action mirred egress redirect dev dummy0 ping 1.1.1.1 After the fix, there are no kmemleak reports with the reproducer. This is in line with what is also done on the ingress side, and from debugging the skb_unref(skb) on dummy xmit and sch_handle_egress() side, it is visible that these are two different skbs with both skb_unref(skb) as true. The two seen skbs are due to mirred doing a skb_clone() internally as use_reinsert is false in tcf_mirred_act() for egress. This was initially reported by Gal. Fixes: e420bed02507 ("bpf: Add fd-based tcx multi-prog infra with link support") Reported-by: Gal Pressman Signed-off-by: Daniel Borkmann Link: https://lore.kernel.org/bpf/bdfc2640-8f65-5b56-4472-db8e2b161aab@nvidia.com Reviewed-by: Simon Horman Signed-off-by: David S. Miller --- net/core/dev.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net') diff --git a/net/core/dev.c b/net/core/dev.c index 17e6281e408c..9f6ed6d97f89 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -4061,6 +4061,7 @@ egress_verdict: case TC_ACT_STOLEN: case TC_ACT_QUEUED: case TC_ACT_TRAP: + consume_skb(skb); *ret = NET_XMIT_SUCCESS; return NULL; } -- cgit v1.2.3