Skip to content

Commit 8285ab7

Browse files
committed
tcx: Fix splat during dev unregister
jira LE-1907 Rebuild_History Non-Buildable kernel-5.14.0-427.18.1.el9_4 commit-author Martin KaFai Lau <[email protected]> commit 079082c Empty-Commit: Cherry-Pick Conflicts during history rebuild. Will be included in final tarball splat. Ref for failed cherry-pick at: ciq/ciq_backports/kernel-5.14.0-427.18.1.el9_4/079082c6.failed During unregister_netdevice_many_notify(), the ordering of our concerned function calls is like this: unregister_netdevice_many_notify dev_shutdown qdisc_put clsact_destroy tcx_uninstall The syzbot reproducer triggered a case that the qdisc refcnt is not zero during dev_shutdown(). tcx_uninstall() will then WARN_ON_ONCE(tcx_entry(entry)->miniq_active) because the miniq is still active and the entry should not be freed. The latter assumed that qdisc destruction happens before tcx teardown. This fix is to avoid tcx_uninstall() doing tcx_entry_free() when the miniq is still alive and let the clsact_destroy() do the free later, so that we do not assume any specific ordering for either of them. If still active, tcx_uninstall() does clear the entry when flushing out the prog/link. clsact_destroy() will then notice the "!tcx_entry_is_active()" and then does the tcx_entry_free() eventually. Fixes: e420bed ("bpf: Add fd-based tcx multi-prog infra with link support") Reported-by: [email protected] Reported-by: Leon Romanovsky <[email protected]> Signed-off-by: Martin KaFai Lau <[email protected]> Co-developed-by: Daniel Borkmann <[email protected]> Signed-off-by: Daniel Borkmann <[email protected]> Tested-by: [email protected] Tested-by: Leon Romanovsky <[email protected]> Link: https://lore.kernel.org/r/222255fe07cb58f15ee662e7ee78328af5b438e4.1690549248.git.daniel@iogearbox.net Signed-off-by: Jakub Kicinski <[email protected]> (cherry picked from commit 079082c) Signed-off-by: Jonathan Maple <[email protected]> # Conflicts: # kernel/bpf/tcx.c
1 parent 5ede909 commit 8285ab7

File tree

1 file changed

+78
-0
lines changed

1 file changed

+78
-0
lines changed
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
tcx: Fix splat during dev unregister
2+
3+
jira LE-1907
4+
Rebuild_History Non-Buildable kernel-5.14.0-427.18.1.el9_4
5+
commit-author Martin KaFai Lau <[email protected]>
6+
commit 079082c60affefeb9d2bd4176a4f2b390a9ccfda
7+
Empty-Commit: Cherry-Pick Conflicts during history rebuild.
8+
Will be included in final tarball splat. Ref for failed cherry-pick at:
9+
ciq/ciq_backports/kernel-5.14.0-427.18.1.el9_4/079082c6.failed
10+
11+
During unregister_netdevice_many_notify(), the ordering of our concerned
12+
function calls is like this:
13+
14+
unregister_netdevice_many_notify
15+
dev_shutdown
16+
qdisc_put
17+
clsact_destroy
18+
tcx_uninstall
19+
20+
The syzbot reproducer triggered a case that the qdisc refcnt is not
21+
zero during dev_shutdown().
22+
23+
tcx_uninstall() will then WARN_ON_ONCE(tcx_entry(entry)->miniq_active)
24+
because the miniq is still active and the entry should not be freed.
25+
The latter assumed that qdisc destruction happens before tcx teardown.
26+
27+
This fix is to avoid tcx_uninstall() doing tcx_entry_free() when the
28+
miniq is still alive and let the clsact_destroy() do the free later, so
29+
that we do not assume any specific ordering for either of them.
30+
31+
If still active, tcx_uninstall() does clear the entry when flushing out
32+
the prog/link. clsact_destroy() will then notice the "!tcx_entry_is_active()"
33+
and then does the tcx_entry_free() eventually.
34+
35+
Fixes: e420bed02507 ("bpf: Add fd-based tcx multi-prog infra with link support")
36+
Reported-by: [email protected]
37+
Reported-by: Leon Romanovsky <[email protected]>
38+
Signed-off-by: Martin KaFai Lau <[email protected]>
39+
Co-developed-by: Daniel Borkmann <[email protected]>
40+
Signed-off-by: Daniel Borkmann <[email protected]>
41+
Tested-by: [email protected]
42+
Tested-by: Leon Romanovsky <[email protected]>
43+
Link: https://lore.kernel.org/r/222255fe07cb58f15ee662e7ee78328af5b438e4.1690549248.git.daniel@iogearbox.net
44+
Signed-off-by: Jakub Kicinski <[email protected]>
45+
(cherry picked from commit 079082c60affefeb9d2bd4176a4f2b390a9ccfda)
46+
Signed-off-by: Jonathan Maple <[email protected]>
47+
48+
# Conflicts:
49+
# kernel/bpf/tcx.c
50+
* Unmerged path kernel/bpf/tcx.c
51+
diff --git a/include/linux/bpf_mprog.h b/include/linux/bpf_mprog.h
52+
index 6feefec43422..0301b983ed43 100644
53+
--- a/include/linux/bpf_mprog.h
54+
+++ b/include/linux/bpf_mprog.h
55+
@@ -256,6 +256,22 @@ static inline void bpf_mprog_entry_copy(struct bpf_mprog_entry *dst,
56+
memcpy(dst->fp_items, src->fp_items, sizeof(src->fp_items));
57+
}
58+
59+
+static inline void bpf_mprog_entry_clear(struct bpf_mprog_entry *dst)
60+
+{
61+
+ memset(dst->fp_items, 0, sizeof(dst->fp_items));
62+
+}
63+
+
64+
+static inline void bpf_mprog_clear_all(struct bpf_mprog_entry *entry,
65+
+ struct bpf_mprog_entry **entry_new)
66+
+{
67+
+ struct bpf_mprog_entry *peer;
68+
+
69+
+ peer = bpf_mprog_peer(entry);
70+
+ bpf_mprog_entry_clear(peer);
71+
+ peer->parent->count = 0;
72+
+ *entry_new = peer;
73+
+}
74+
+
75+
static inline void bpf_mprog_entry_grow(struct bpf_mprog_entry *entry, int idx)
76+
{
77+
int total = bpf_mprog_total(entry);
78+
* Unmerged path kernel/bpf/tcx.c

0 commit comments

Comments
 (0)