From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Jiri Olsa <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andriin@fb.com>,
Network Development <netdev@vger.kernel.org>,
bpf <bpf@vger.kernel.org>, Martin KaFai Lau <kafai@fb.com>,
Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
John Fastabend <john.fastabend@gmail.com>,
KP Singh <kpsingh@chromium.org>
Subject: Re: [PATCH bpf-next] selftests/bpf: Check trampoline execution in d_path test
Date: Thu, 10 Sep 2020 17:46:21 -0700 [thread overview]
Message-ID: <CAADnVQJ0FchoPqNWm+dEppyij-MOvvEG_trEfyrHdabtcEuZGg@mail.gmail.com> (raw)
In-Reply-To: <20200910122224.1683258-1-jolsa@kernel.org>
On Thu, Sep 10, 2020 at 5:22 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Some kernels builds might inline vfs_getattr call within
> fstat syscall code path, so fentry/vfs_getattr trampoline
> is not called.
>
> I'm not sure how to handle this in some generic way other
> than use some other function, but that might get inlined at
> some point as well.
It's great that we had the test and it failed.
Doing the test skipping will only hide the problem.
Please don't do it here and in the future.
Instead let's figure out the real solution.
Assuming that vfs_getattr was added to btf_allowlist_d_path
for a reason we have to make this introspection place
reliable regardless of compiler inlining decisions.
We can mark it as 'noinline', but that's undesirable.
I suggest we remove it from the allowlist and replace it with
security_inode_getattr.
I think that is a better long term fix.
While at it I would apply the same critical thinking to other
functions in the allowlist. They might suffer the same issue.
So s/vfs_truncate/security_path_truncate/ and so on?
Things won't work when CONFIG_SECURITY is off, but that is a rare kernel config?
Or add both security_* and vfs_* variants and switch tests to use security_* ?
but it feels fragile to allow inline-able funcs in allowlist.
next prev parent reply other threads:[~2020-09-11 0:46 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-10 12:22 [PATCH bpf-next] selftests/bpf: Check trampoline execution in d_path test Jiri Olsa
2020-09-10 22:22 ` Andrii Nakryiko
2020-09-11 13:04 ` Jiri Olsa
2020-09-11 0:46 ` Alexei Starovoitov [this message]
2020-09-11 13:15 ` Jiri Olsa
2020-09-15 2:30 ` Alexei Starovoitov
2020-09-15 9:12 ` Jiri Olsa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAADnVQJ0FchoPqNWm+dEppyij-MOvvEG_trEfyrHdabtcEuZGg@mail.gmail.com \
--to=alexei.starovoitov@gmail.com \
--cc=andriin@fb.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kafai@fb.com \
--cc=kpsingh@chromium.org \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.com \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).