Re: [PATCH 2/3] perf test: fix sys_enter_openat event test for musl

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.

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