* [PATCH bpf-next 1/2] bpf: Fix mprog detachment for empty mprog entry
@ 2023-08-04 13:11 Daniel Borkmann
2023-08-04 13:11 ` [PATCH bpf-next 2/2] selftests/bpf: Add test for detachment on " Daniel Borkmann
2023-08-04 16:50 ` [PATCH bpf-next 1/2] bpf: Fix mprog detachment for " patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Daniel Borkmann @ 2023-08-04 13:11 UTC (permalink / raw)
To: martin.lau; +Cc: bpf, Daniel Borkmann, syzbot+0c06ba0f831fe07a8f27
syzbot reported an UBSAN array-index-out-of-bounds access in bpf_mprog_read()
upon bpf_mprog_detach(). While it did not have a reproducer, I was able to
manually reproduce through an empty mprog entry which just has miniq present.
The latter is important given otherwise we get an ENOENT error as tcx detaches
the whole mprog entry. The index 4294967295 was triggered via NULL dtuple.prog
which then attempts to detach from the back. bpf_mprog_fetch() in this case
did hit the idx == total and therefore tried to grab the entry at idx -1.
Fix it by adding an explicit bpf_mprog_total() check in bpf_mprog_detach() and
bail out early with ENOENT.
Fixes: 053c8e1f235d ("bpf: Add generic attach/detach/query API for multi-progs")
Reported-by: syzbot+0c06ba0f831fe07a8f27@syzkaller.appspotmail.com
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
kernel/bpf/mprog.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/bpf/mprog.c b/kernel/bpf/mprog.c
index f7816d2bc3e4..32d2c4829eb8 100644
--- a/kernel/bpf/mprog.c
+++ b/kernel/bpf/mprog.c
@@ -337,6 +337,8 @@ int bpf_mprog_detach(struct bpf_mprog_entry *entry,
return -EINVAL;
if (revision && revision != bpf_mprog_revision(entry))
return -ESTALE;
+ if (!bpf_mprog_total(entry))
+ return -ENOENT;
ret = bpf_mprog_tuple_relative(&rtuple, id_or_fd, flags,
prog ? prog->type :
BPF_PROG_TYPE_UNSPEC);
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH bpf-next 2/2] selftests/bpf: Add test for detachment on empty mprog entry
2023-08-04 13:11 [PATCH bpf-next 1/2] bpf: Fix mprog detachment for empty mprog entry Daniel Borkmann
@ 2023-08-04 13:11 ` Daniel Borkmann
2023-08-04 16:50 ` [PATCH bpf-next 1/2] bpf: Fix mprog detachment for " patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Daniel Borkmann @ 2023-08-04 13:11 UTC (permalink / raw)
To: martin.lau; +Cc: bpf, Daniel Borkmann
Add a detachment test case with miniq present to assert that with and
without the miniq we get the same error.
# ./test_progs -t tc_opts
#244 tc_opts_after:OK
#245 tc_opts_append:OK
#246 tc_opts_basic:OK
#247 tc_opts_before:OK
#248 tc_opts_chain_classic:OK
#249 tc_opts_delete_empty:OK
#250 tc_opts_demixed:OK
#251 tc_opts_detach:OK
#252 tc_opts_detach_after:OK
#253 tc_opts_detach_before:OK
#254 tc_opts_dev_cleanup:OK
#255 tc_opts_invalid:OK
#256 tc_opts_mixed:OK
#257 tc_opts_prepend:OK
#258 tc_opts_replace:OK
#259 tc_opts_revision:OK
Summary: 16/0 PASSED, 0 SKIPPED, 0 FAILED
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
.../selftests/bpf/prog_tests/tc_opts.c | 31 +++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/tc_opts.c b/tools/testing/selftests/bpf/prog_tests/tc_opts.c
index 7914100f9b46..39bd253e41aa 100644
--- a/tools/testing/selftests/bpf/prog_tests/tc_opts.c
+++ b/tools/testing/selftests/bpf/prog_tests/tc_opts.c
@@ -2237,3 +2237,34 @@ void serial_test_tc_opts_detach_after(void)
test_tc_opts_detach_after_target(BPF_TCX_INGRESS);
test_tc_opts_detach_after_target(BPF_TCX_EGRESS);
}
+
+static void test_tc_opts_delete_empty(int target, bool chain_tc_old)
+{
+ LIBBPF_OPTS(bpf_tc_hook, tc_hook, .ifindex = loopback);
+ LIBBPF_OPTS(bpf_prog_detach_opts, optd);
+ int err;
+
+ assert_mprog_count(target, 0);
+ if (chain_tc_old) {
+ tc_hook.attach_point = target == BPF_TCX_INGRESS ?
+ BPF_TC_INGRESS : BPF_TC_EGRESS;
+ err = bpf_tc_hook_create(&tc_hook);
+ ASSERT_OK(err, "bpf_tc_hook_create");
+ __assert_mprog_count(target, 0, true, loopback);
+ }
+ err = bpf_prog_detach_opts(0, loopback, target, &optd);
+ ASSERT_EQ(err, -ENOENT, "prog_detach");
+ if (chain_tc_old) {
+ tc_hook.attach_point = BPF_TC_INGRESS | BPF_TC_EGRESS;
+ bpf_tc_hook_destroy(&tc_hook);
+ }
+ assert_mprog_count(target, 0);
+}
+
+void serial_test_tc_opts_delete_empty(void)
+{
+ test_tc_opts_delete_empty(BPF_TCX_INGRESS, false);
+ test_tc_opts_delete_empty(BPF_TCX_EGRESS, false);
+ test_tc_opts_delete_empty(BPF_TCX_INGRESS, true);
+ test_tc_opts_delete_empty(BPF_TCX_EGRESS, true);
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: Fix mprog detachment for empty mprog entry
2023-08-04 13:11 [PATCH bpf-next 1/2] bpf: Fix mprog detachment for empty mprog entry Daniel Borkmann
2023-08-04 13:11 ` [PATCH bpf-next 2/2] selftests/bpf: Add test for detachment on " Daniel Borkmann
@ 2023-08-04 16:50 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-08-04 16:50 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: martin.lau, bpf, syzbot+0c06ba0f831fe07a8f27
Hello:
This series was applied to bpf/bpf-next.git (master)
by Martin KaFai Lau <martin.lau@kernel.org>:
On Fri, 4 Aug 2023 15:11:11 +0200 you wrote:
> syzbot reported an UBSAN array-index-out-of-bounds access in bpf_mprog_read()
> upon bpf_mprog_detach(). While it did not have a reproducer, I was able to
> manually reproduce through an empty mprog entry which just has miniq present.
>
> The latter is important given otherwise we get an ENOENT error as tcx detaches
> the whole mprog entry. The index 4294967295 was triggered via NULL dtuple.prog
> which then attempts to detach from the back. bpf_mprog_fetch() in this case
> did hit the idx == total and therefore tried to grab the entry at idx -1.
>
> [...]
Here is the summary with links:
- [bpf-next,1/2] bpf: Fix mprog detachment for empty mprog entry
https://git.kernel.org/bpf/bpf-next/c/d210f9735e13
- [bpf-next,2/2] selftests/bpf: Add test for detachment on empty mprog entry
https://git.kernel.org/bpf/bpf-next/c/21ce6abe178a
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-08-04 16:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-04 13:11 [PATCH bpf-next 1/2] bpf: Fix mprog detachment for empty mprog entry Daniel Borkmann
2023-08-04 13:11 ` [PATCH bpf-next 2/2] selftests/bpf: Add test for detachment on " Daniel Borkmann
2023-08-04 16:50 ` [PATCH bpf-next 1/2] bpf: Fix mprog detachment for " patchwork-bot+netdevbpf
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).