Re: [PATCH bpf-next v3 12/17] bpf: Disallow fentry/fexit/freplace for exception callbacks

[Kumar Kartikeya Dwivedi <memxor@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1795 bytes --]

On Sat, 16 Sept 2023 at 19:30, Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:
>
> On Sat, 16 Sept 2023 at 18:44, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Thu, Sep 14, 2023 at 5:13 AM Kumar Kartikeya Dwivedi
> > <memxor@gmail.com> wrote:
> > > > >                       }
> > > > > +                     if (aux->func && aux->func[subprog]->aux->exception_cb) {
> > > > > +                             bpf_log(log,
> > > > > +                                     "Extension programs cannot replace exception callback\n");
> > > > > +                             return -EINVAL;
> > > > > +                     }
> > > >
> > > > This check is redundant because you already did this check above if (prog_extension branch)
> > > > Remove this as it will never be reached.
> > > >
> > >
> > > Good catch, will fix it in v4.
> >
> > No worries. I fixed this duplicate check while applying.
> > Everything else can be addressed in the follow ups.
> >
> > This spam is a bit annoying:
> > $ ./test_progs -t exceptions
> > func#0 @0
> > FENTRY/FEXIT programs cannot attach to exception callback
> > processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0
> > peak_states 0 mark_read 0
> >
> > func#0 @0
> > FENTRY/FEXIT programs cannot attach to exception callback
> > processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0
> > peak_states 0 mark_read 0
>
> Thanks for fixing it while applying. I will send a follow up to
> silence these logs today.

For some reason, I don't seem to see these when just running
./test_progs -t exceptions.
I am not sure what I'm doing differently when running the selftests.
A bit weird, but anyway, just guessing the cause, do you see them when
you apply this?

[-- Attachment #2: 0001-selftests-bpf-Printing-log-buffer-for-exceptions-tes.patch --]
[-- Type: text/x-patch, Size: 1665 bytes --]

From 16c75737c44b5719f5acc1819495d08169fd7e76 Mon Sep 17 00:00:00 2001
From: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Date: Sat, 16 Sep 2023 21:30:12 +0200
Subject: [PATCH bpf-next] selftests/bpf: Printing log buffer for exceptions test only
 on failure

Alexei reported seeing log messages for some test cases even though we
just wanted to match the error string from the verifier. Move the
printing of the log buffer to a guarded condition so that we only print
it when we fail to match on the expected string in the log buffer,
preventing unneeded output when running the test.

Reported-by: Alexei Starovoitov <ast@kernel.org>
Fixes: d2a93715bfb0 ("selftests/bpf: Add tests for BPF exceptions")
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 tools/testing/selftests/bpf/prog_tests/exceptions.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/exceptions.c b/tools/testing/selftests/bpf/prog_tests/exceptions.c
index 5663e427dc00..516f4a13013c 100644
--- a/tools/testing/selftests/bpf/prog_tests/exceptions.c
+++ b/tools/testing/selftests/bpf/prog_tests/exceptions.c
@@ -103,9 +103,10 @@ static void test_exceptions_success(void)
 			goto done;						  \
 		}								  \
 		if (load_ret != 0) {						  \
-			printf("%s\n", log_buf);				  \
-			if (!ASSERT_OK_PTR(strstr(log_buf, msg), "strstr"))	  \
+			if (!ASSERT_OK_PTR(strstr(log_buf, msg), "strstr")) {	  \
+				printf("%s\n", log_buf);			  \
 				goto done;					  \
+			}							  \
 		}								  \
 		if (!load_ret && attach_err) {					  \
 			if (!ASSERT_ERR_PTR(link = bpf_program__attach(prog), "attach err")) \
--
2.41.0