* [PATCH bpf v2] flow_dissector: Drop BPF flow dissector prog ref on netns cleanup
@ 2020-05-21 8:34 Jakub Sitnicki
2020-05-21 15:21 ` sdf
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Sitnicki @ 2020-05-21 8:34 UTC (permalink / raw)
To: bpf; +Cc: netdev, kernel-team, Stanislav Fomichev, Alexei Starovoitov
When attaching a flow dissector program to a network namespace with
bpf(BPF_PROG_ATTACH, ...) we grab a reference to bpf_prog.
If netns gets destroyed while a flow dissector is still attached, and there
are no other references to the prog, we leak the reference and the program
remains loaded.
Leak can be reproduced by running flow dissector tests from selftests/bpf:
# bpftool prog list
# ./test_flow_dissector.sh
...
selftests: test_flow_dissector [PASS]
# bpftool prog list
4: flow_dissector name _dissect tag e314084d332a5338 gpl
loaded_at 2020-05-20T18:50:53+0200 uid 0
xlated 552B jited 355B memlock 4096B map_ids 3,4
btf_id 4
#
Fix it by detaching the flow dissector program when netns is going away.
Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
---
Notes:
v2:
- Share code between pre_exit and prog_detach callbacks (Stanislav)
net/core/flow_dissector.c | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 3eff84824c8b..3ad723b2e299 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -160,12 +160,10 @@ int skb_flow_dissector_bpf_prog_attach(const union bpf_attr *attr,
return ret;
}
-int skb_flow_dissector_bpf_prog_detach(const union bpf_attr *attr)
+static int flow_dissector_bpf_prog_detach(struct net *net)
{
struct bpf_prog *attached;
- struct net *net;
- net = current->nsproxy->net_ns;
mutex_lock(&flow_dissector_mutex);
attached = rcu_dereference_protected(net->flow_dissector_prog,
lockdep_is_held(&flow_dissector_mutex));
@@ -179,6 +177,24 @@ int skb_flow_dissector_bpf_prog_detach(const union bpf_attr *attr)
return 0;
}
+int skb_flow_dissector_bpf_prog_detach(const union bpf_attr *attr)
+{
+ return flow_dissector_bpf_prog_detach(current->nsproxy->net_ns);
+}
+
+static void __net_exit flow_dissector_pernet_pre_exit(struct net *net)
+{
+ /* We're not racing with attach/detach because there are no
+ * references to netns left when pre_exit gets called.
+ */
+ if (rcu_access_pointer(net->flow_dissector_prog))
+ flow_dissector_bpf_prog_detach(net);
+}
+
+static struct pernet_operations flow_dissector_pernet_ops __net_initdata = {
+ .pre_exit = flow_dissector_pernet_pre_exit,
+};
+
/**
* __skb_flow_get_ports - extract the upper layer ports and return them
* @skb: sk_buff to extract the ports from
@@ -1827,6 +1843,8 @@ EXPORT_SYMBOL(flow_keys_basic_dissector);
static int __init init_default_flow_dissectors(void)
{
+ int err;
+
skb_flow_dissector_init(&flow_keys_dissector,
flow_keys_dissector_keys,
ARRAY_SIZE(flow_keys_dissector_keys));
@@ -1836,7 +1854,11 @@ static int __init init_default_flow_dissectors(void)
skb_flow_dissector_init(&flow_keys_basic_dissector,
flow_keys_basic_dissector_keys,
ARRAY_SIZE(flow_keys_basic_dissector_keys));
- return 0;
+
+ err = register_pernet_subsys(&flow_dissector_pernet_ops);
+
+ WARN_ON(err);
+ return err;
}
core_initcall(init_default_flow_dissectors);
--
2.25.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf v2] flow_dissector: Drop BPF flow dissector prog ref on netns cleanup
2020-05-21 8:34 [PATCH bpf v2] flow_dissector: Drop BPF flow dissector prog ref on netns cleanup Jakub Sitnicki
@ 2020-05-21 15:21 ` sdf
2020-05-21 18:14 ` Alexei Starovoitov
0 siblings, 1 reply; 3+ messages in thread
From: sdf @ 2020-05-21 15:21 UTC (permalink / raw)
To: Jakub Sitnicki; +Cc: bpf, netdev, kernel-team, Alexei Starovoitov
On 05/21, Jakub Sitnicki wrote:
> When attaching a flow dissector program to a network namespace with
> bpf(BPF_PROG_ATTACH, ...) we grab a reference to bpf_prog.
> If netns gets destroyed while a flow dissector is still attached, and
> there
> are no other references to the prog, we leak the reference and the program
> remains loaded.
> Leak can be reproduced by running flow dissector tests from selftests/bpf:
> # bpftool prog list
> # ./test_flow_dissector.sh
> ...
> selftests: test_flow_dissector [PASS]
> # bpftool prog list
> 4: flow_dissector name _dissect tag e314084d332a5338 gpl
> loaded_at 2020-05-20T18:50:53+0200 uid 0
> xlated 552B jited 355B memlock 4096B map_ids 3,4
> btf_id 4
> #
> Fix it by detaching the flow dissector program when netns is going away.
> Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
Thank you!
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf v2] flow_dissector: Drop BPF flow dissector prog ref on netns cleanup
2020-05-21 15:21 ` sdf
@ 2020-05-21 18:14 ` Alexei Starovoitov
0 siblings, 0 replies; 3+ messages in thread
From: Alexei Starovoitov @ 2020-05-21 18:14 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: Jakub Sitnicki, bpf, Network Development, kernel-team
On Thu, May 21, 2020 at 8:21 AM <sdf@google.com> wrote:
>
> On 05/21, Jakub Sitnicki wrote:
> > When attaching a flow dissector program to a network namespace with
> > bpf(BPF_PROG_ATTACH, ...) we grab a reference to bpf_prog.
>
> > If netns gets destroyed while a flow dissector is still attached, and
> > there
> > are no other references to the prog, we leak the reference and the program
> > remains loaded.
>
> > Leak can be reproduced by running flow dissector tests from selftests/bpf:
>
> > # bpftool prog list
> > # ./test_flow_dissector.sh
> > ...
> > selftests: test_flow_dissector [PASS]
> > # bpftool prog list
> > 4: flow_dissector name _dissect tag e314084d332a5338 gpl
> > loaded_at 2020-05-20T18:50:53+0200 uid 0
> > xlated 552B jited 355B memlock 4096B map_ids 3,4
> > btf_id 4
> > #
>
> > Fix it by detaching the flow dissector program when netns is going away.
>
> > Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
> > Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
> Reviewed-by: Stanislav Fomichev <sdf@google.com>
Applied. Thanks
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-21 18:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-21 8:34 [PATCH bpf v2] flow_dissector: Drop BPF flow dissector prog ref on netns cleanup Jakub Sitnicki
2020-05-21 15:21 ` sdf
2020-05-21 18:14 ` Alexei Starovoitov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).