edgeos: Fix kernel memory leak
This patch has been reviewed and accepted by the netfilter maintainers,
but it will likely take some time before it makes its way back to the
5.15 stable tree. Let's include it earlier since we are affected by it.
BUG=b/239435672
TEST=memory leak fixed
RELEASE_NOTE=none
Change-Id: I9ead8ccdccb43a2b3d9b37061bf591156669fbe4
Reviewed-on: https://cos-review.googlesource.com/c/cos/overlays/board-overlays/+/36595
Reviewed-by: Joshua Emele <jemele@google.com>
Reviewed-by: Robert Kolchmeyer <rkolchmeyer@google.com>
Tested-by: Cusky Presubmit Bot <presubmit@cos-infra-prod.iam.gserviceaccount.com>
Reviewed-by: Ian Coolidge <icoolidge@google.com>
diff --git a/project-edgeos/sys-kernel/lakitu-kernel-5_15/files/0001-br_netfilter-Drop-dst-refernces-before-reseting.patch b/project-edgeos/sys-kernel/lakitu-kernel-5_15/files/0001-br_netfilter-Drop-dst-refernces-before-reseting.patch
new file mode 100644
index 0000000..1f41a323
--- /dev/null
+++ b/project-edgeos/sys-kernel/lakitu-kernel-5_15/files/0001-br_netfilter-Drop-dst-refernces-before-reseting.patch
@@ -0,0 +1,91 @@
+From d047283a7034140ea5da759a494fd2274affdd46 Mon Sep 17 00:00:00 2001
+From: Harsh Modi <harshmodi@google.com>
+Date: Tue, 30 Aug 2022 22:36:03 -0700
+Subject: netfilter: br_netfilter: Drop dst references before setting.
+
+The IPv6 path already drops dst in the daddr changed case, but the IPv4
+path does not. This change makes the two code paths consistent.
+
+Further, it is possible that there is already a metadata_dst allocated from
+ingress that might already be attached to skbuff->dst while following
+the bridge path. If it is not released before setting a new
+metadata_dst, it will be leaked. This is similar to what is done in
+bpf_set_tunnel_key() or ip6_route_input().
+
+It is important to note that the memory being leaked is not the dst
+being set in the bridge code, but rather memory allocated from some
+other code path that is not being freed correctly before the skb dst is
+overwritten.
+
+An example of the leakage fixed by this commit found using kmemleak:
+
+unreferenced object 0xffff888010112b00 (size 256):
+ comm "softirq", pid 0, jiffies 4294762496 (age 32.012s)
+ hex dump (first 32 bytes):
+ 00 00 00 00 00 00 00 00 80 16 f1 83 ff ff ff ff ................
+ e1 4e f6 82 ff ff ff ff 00 00 00 00 00 00 00 00 .N..............
+ backtrace:
+ [<00000000d79567ea>] metadata_dst_alloc+0x1b/0xe0
+ [<00000000be113e13>] udp_tun_rx_dst+0x174/0x1f0
+ [<00000000a36848f4>] geneve_udp_encap_recv+0x350/0x7b0
+ [<00000000d4afb476>] udp_queue_rcv_one_skb+0x380/0x560
+ [<00000000ac064aea>] udp_unicast_rcv_skb+0x75/0x90
+ [<000000009a8ee8c5>] ip_protocol_deliver_rcu+0xd8/0x230
+ [<00000000ef4980bb>] ip_local_deliver_finish+0x7a/0xa0
+ [<00000000d7533c8c>] __netif_receive_skb_one_core+0x89/0xa0
+ [<00000000a879497d>] process_backlog+0x93/0x190
+ [<00000000e41ade9f>] __napi_poll+0x28/0x170
+ [<00000000b4c0906b>] net_rx_action+0x14f/0x2a0
+ [<00000000b20dd5d4>] __do_softirq+0xf4/0x305
+ [<000000003a7d7e15>] __irq_exit_rcu+0xc3/0x140
+ [<00000000968d39a2>] sysvec_apic_timer_interrupt+0x9e/0xc0
+ [<000000009e920794>] asm_sysvec_apic_timer_interrupt+0x16/0x20
+ [<000000008942add0>] native_safe_halt+0x13/0x20
+
+Florian Westphal says: "Original code was likely fine because nothing
+ever did set a skb->dst entry earlier than bridge in those days."
+
+Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
+Signed-off-by: Harsh Modi <harshmodi@google.com>
+Acked-by: Florian Westphal <fw@strlen.de>
+Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
+---
+ net/bridge/br_netfilter_hooks.c | 2 ++
+ net/bridge/br_netfilter_ipv6.c | 1 +
+ 2 files changed, 3 insertions(+)
+
+diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
+index ff47790366497..f20f4373ff408 100644
+--- a/net/bridge/br_netfilter_hooks.c
++++ b/net/bridge/br_netfilter_hooks.c
+@@ -384,6 +384,7 @@ static int br_nf_pre_routing_finish(struct net *net, struct sock *sk, struct sk_
+ /* - Bridged-and-DNAT'ed traffic doesn't
+ * require ip_forwarding. */
+ if (rt->dst.dev == dev) {
++ skb_dst_drop(skb);
+ skb_dst_set(skb, &rt->dst);
+ goto bridged_dnat;
+ }
+@@ -413,6 +414,7 @@ bridged_dnat:
+ kfree_skb(skb);
+ return 0;
+ }
++ skb_dst_drop(skb);
+ skb_dst_set_noref(skb, &rt->dst);
+ }
+
+diff --git a/net/bridge/br_netfilter_ipv6.c b/net/bridge/br_netfilter_ipv6.c
+index e4e0c836c3f51..6b07f30675bb0 100644
+--- a/net/bridge/br_netfilter_ipv6.c
++++ b/net/bridge/br_netfilter_ipv6.c
+@@ -197,6 +197,7 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc
+ kfree_skb(skb);
+ return 0;
+ }
++ skb_dst_drop(skb);
+ skb_dst_set_noref(skb, &rt->dst);
+ }
+
+--
+cgit
+
diff --git a/project-edgeos/sys-kernel/lakitu-kernel-5_15/lakitu-kernel-5_15-5.15.61-r6.ebuild b/project-edgeos/sys-kernel/lakitu-kernel-5_15/lakitu-kernel-5_15-5.15.61-r7.ebuild
similarity index 100%
rename from project-edgeos/sys-kernel/lakitu-kernel-5_15/lakitu-kernel-5_15-5.15.61-r6.ebuild
rename to project-edgeos/sys-kernel/lakitu-kernel-5_15/lakitu-kernel-5_15-5.15.61-r7.ebuild
diff --git a/project-edgeos/sys-kernel/lakitu-kernel-rt-5_15/files/0003-br_netfilter-Drop-dst-refernces-before-reseting.patch b/project-edgeos/sys-kernel/lakitu-kernel-rt-5_15/files/0003-br_netfilter-Drop-dst-refernces-before-reseting.patch
new file mode 100644
index 0000000..1f41a323
--- /dev/null
+++ b/project-edgeos/sys-kernel/lakitu-kernel-rt-5_15/files/0003-br_netfilter-Drop-dst-refernces-before-reseting.patch
@@ -0,0 +1,91 @@
+From d047283a7034140ea5da759a494fd2274affdd46 Mon Sep 17 00:00:00 2001
+From: Harsh Modi <harshmodi@google.com>
+Date: Tue, 30 Aug 2022 22:36:03 -0700
+Subject: netfilter: br_netfilter: Drop dst references before setting.
+
+The IPv6 path already drops dst in the daddr changed case, but the IPv4
+path does not. This change makes the two code paths consistent.
+
+Further, it is possible that there is already a metadata_dst allocated from
+ingress that might already be attached to skbuff->dst while following
+the bridge path. If it is not released before setting a new
+metadata_dst, it will be leaked. This is similar to what is done in
+bpf_set_tunnel_key() or ip6_route_input().
+
+It is important to note that the memory being leaked is not the dst
+being set in the bridge code, but rather memory allocated from some
+other code path that is not being freed correctly before the skb dst is
+overwritten.
+
+An example of the leakage fixed by this commit found using kmemleak:
+
+unreferenced object 0xffff888010112b00 (size 256):
+ comm "softirq", pid 0, jiffies 4294762496 (age 32.012s)
+ hex dump (first 32 bytes):
+ 00 00 00 00 00 00 00 00 80 16 f1 83 ff ff ff ff ................
+ e1 4e f6 82 ff ff ff ff 00 00 00 00 00 00 00 00 .N..............
+ backtrace:
+ [<00000000d79567ea>] metadata_dst_alloc+0x1b/0xe0
+ [<00000000be113e13>] udp_tun_rx_dst+0x174/0x1f0
+ [<00000000a36848f4>] geneve_udp_encap_recv+0x350/0x7b0
+ [<00000000d4afb476>] udp_queue_rcv_one_skb+0x380/0x560
+ [<00000000ac064aea>] udp_unicast_rcv_skb+0x75/0x90
+ [<000000009a8ee8c5>] ip_protocol_deliver_rcu+0xd8/0x230
+ [<00000000ef4980bb>] ip_local_deliver_finish+0x7a/0xa0
+ [<00000000d7533c8c>] __netif_receive_skb_one_core+0x89/0xa0
+ [<00000000a879497d>] process_backlog+0x93/0x190
+ [<00000000e41ade9f>] __napi_poll+0x28/0x170
+ [<00000000b4c0906b>] net_rx_action+0x14f/0x2a0
+ [<00000000b20dd5d4>] __do_softirq+0xf4/0x305
+ [<000000003a7d7e15>] __irq_exit_rcu+0xc3/0x140
+ [<00000000968d39a2>] sysvec_apic_timer_interrupt+0x9e/0xc0
+ [<000000009e920794>] asm_sysvec_apic_timer_interrupt+0x16/0x20
+ [<000000008942add0>] native_safe_halt+0x13/0x20
+
+Florian Westphal says: "Original code was likely fine because nothing
+ever did set a skb->dst entry earlier than bridge in those days."
+
+Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
+Signed-off-by: Harsh Modi <harshmodi@google.com>
+Acked-by: Florian Westphal <fw@strlen.de>
+Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
+---
+ net/bridge/br_netfilter_hooks.c | 2 ++
+ net/bridge/br_netfilter_ipv6.c | 1 +
+ 2 files changed, 3 insertions(+)
+
+diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
+index ff47790366497..f20f4373ff408 100644
+--- a/net/bridge/br_netfilter_hooks.c
++++ b/net/bridge/br_netfilter_hooks.c
+@@ -384,6 +384,7 @@ static int br_nf_pre_routing_finish(struct net *net, struct sock *sk, struct sk_
+ /* - Bridged-and-DNAT'ed traffic doesn't
+ * require ip_forwarding. */
+ if (rt->dst.dev == dev) {
++ skb_dst_drop(skb);
+ skb_dst_set(skb, &rt->dst);
+ goto bridged_dnat;
+ }
+@@ -413,6 +414,7 @@ bridged_dnat:
+ kfree_skb(skb);
+ return 0;
+ }
++ skb_dst_drop(skb);
+ skb_dst_set_noref(skb, &rt->dst);
+ }
+
+diff --git a/net/bridge/br_netfilter_ipv6.c b/net/bridge/br_netfilter_ipv6.c
+index e4e0c836c3f51..6b07f30675bb0 100644
+--- a/net/bridge/br_netfilter_ipv6.c
++++ b/net/bridge/br_netfilter_ipv6.c
+@@ -197,6 +197,7 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc
+ kfree_skb(skb);
+ return 0;
+ }
++ skb_dst_drop(skb);
+ skb_dst_set_noref(skb, &rt->dst);
+ }
+
+--
+cgit
+
diff --git a/project-edgeos/sys-kernel/lakitu-kernel-rt-5_15/lakitu-kernel-rt-5_15-5.15.55-r5.ebuild b/project-edgeos/sys-kernel/lakitu-kernel-rt-5_15/lakitu-kernel-rt-5_15-5.15.55-r6.ebuild
similarity index 100%
rename from project-edgeos/sys-kernel/lakitu-kernel-rt-5_15/lakitu-kernel-rt-5_15-5.15.55-r5.ebuild
rename to project-edgeos/sys-kernel/lakitu-kernel-rt-5_15/lakitu-kernel-rt-5_15-5.15.55-r6.ebuild
