Re: [PATCH 2/3] perf test: fix sys_enter_openat event test for musl
Martin Kaiser <[email protected]>
| Newsgroups | dev.linux.lists.sashiko,org.kernel.vger.linux-perf-users |
|---|---|
| Message-ID | <[email protected]> |
Thus wrote Namhyung Kim ([email protected]): > On Mon, Apr 27, 2026 at 09:49:37AM +0000, [email protected] wrote: > > Sashiko AI review found 1 potential issue(s): > > - [Medium] The updated bitwise check `(tp_flags & flags) != flags` fails to verify the `O_RDONLY` access mode and permits arbitrary unrelated flags, severely weakening the test. > > -- > > commit 0700122c98ba7f263a329cd8f9fbeef95b67603e > > Author: Martin Kaiser <[email protected]> > > perf test: fix sys_enter_openat event test for musl > > This commit updates the sys_enter_openat test to allow additional flags in the > > captured event. It replaces the strict equality check with a bitwise mask check > > to accommodate the O_LARGEFILE flag implicitly added by musl libc. > > > diff --git a/tools/perf/tests/openat-syscall-tp-fields.c b/tools/perf/tests/openat-syscall-tp-fields.c > > > --- a/tools/perf/tests/openat-syscall-tp-fields.c > > > +++ b/tools/perf/tests/openat-syscall-tp-fields.c > > [ ... ] > > > @@ -120,7 +120,8 @@ static int test__syscall_openat_tp_fields(struct test_suite *test __maybe_unused > > > tp_flags = evsel__intval(evsel, &sample, "flags"); > > > perf_sample__exit(&sample); > > > - if (flags != tp_flags) { > > > + /* C library wrapper may set additional flags */ > > > + if ((tp_flags & flags) != flags) { > > Does this check still verify the O_RDONLY access mode? > > Since O_RDONLY is defined as 0, the flags variable evaluates to exactly > > O_DIRECTORY. This means the condition functionally becomes > > (tp_flags & O_DIRECTORY) != O_DIRECTORY. > > Does this unintentionally permit other access modes like O_WRONLY or O_RDWR, > > and ignore other unrelated flags as long as O_DIRECTORY is set? > I think it's ok for tp_flags to have more flags. I added another check for the access mode flags in v2. This addresses the sashiko comment and allows for additional flags in the syscall wrapper. Best regards, Martin > Acked-by: Namhyung Kim <[email protected]> > Thanks, > Namhyung > > > pr_debug("%s: Expected flags=%#x, got %#x\n", > > > __func__, flags, tp_flags); > > > goto out_delete_evlist; > > -- > > Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2