[PATCH bpf-next] selftests/bpf: Check trampoline execution in d_path test

[PATCH bpf-next] selftests/bpf: Check trampoline execution in d_path test

[Jiri Olsa]

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.

Adding flags that indicate trampolines were called and failing
the test if neither of them got called.

  $ sudo ./test_progs -t d_path
  test_d_path:PASS:setup 0 nsec
  ...
  trigger_fstat_events:PASS:trigger 0 nsec
  test_d_path:FAIL:124 trampolines not called
  #22 d_path:FAIL
  Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED

If only one trampoline is called, it's still enough to test
the helper, so only warn about missing trampoline call and
continue in test.

  $ sudo ./test_progs -t d_path -v
  test_d_path:PASS:setup 0 nsec
  ...
  trigger_fstat_events:PASS:trigger 0 nsec
  fentry/vfs_getattr not called
  #22 d_path:OK
  Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 .../testing/selftests/bpf/prog_tests/d_path.c | 25 +++++++++++++++----
 .../testing/selftests/bpf/progs/test_d_path.c |  7 ++++++
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
index fc12e0d445ff..ec15f7d1dd0a 100644
--- a/tools/testing/selftests/bpf/prog_tests/d_path.c
+++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
@@ -120,26 +120,41 @@ void test_d_path(void)
 	if (err < 0)
 		goto cleanup;
 
+	if (!bss->called_stat && !bss->called_close) {
+		PRINT_FAIL("trampolines not called\n");
+		goto cleanup;
+	}
+
+	if (!bss->called_stat) {
+		fprintf(stdout, "fentry/vfs_getattr not called\n");
+		goto cleanup;
+	}
+
+	if (!bss->called_close) {
+		fprintf(stdout, "fentry/filp_close not called\n");
+		goto cleanup;
+	}
+
 	for (int i = 0; i < MAX_FILES; i++) {
-		CHECK(strncmp(src.paths[i], bss->paths_stat[i], MAX_PATH_LEN),
+		CHECK(bss->called_stat && strncmp(src.paths[i], bss->paths_stat[i], MAX_PATH_LEN),
 		      "check",
 		      "failed to get stat path[%d]: %s vs %s\n",
 		      i, src.paths[i], bss->paths_stat[i]);
-		CHECK(strncmp(src.paths[i], bss->paths_close[i], MAX_PATH_LEN),
+		CHECK(bss->called_close && strncmp(src.paths[i], bss->paths_close[i], MAX_PATH_LEN),
 		      "check",
 		      "failed to get close path[%d]: %s vs %s\n",
 		      i, src.paths[i], bss->paths_close[i]);
 		/* The d_path helper returns size plus NUL char, hence + 1 */
-		CHECK(bss->rets_stat[i] != strlen(bss->paths_stat[i]) + 1,
+		CHECK(bss->called_stat && bss->rets_stat[i] != strlen(bss->paths_stat[i]) + 1,
 		      "check",
 		      "failed to match stat return [%d]: %d vs %zd [%s]\n",
 		      i, bss->rets_stat[i], strlen(bss->paths_stat[i]) + 1,
 		      bss->paths_stat[i]);
-		CHECK(bss->rets_close[i] != strlen(bss->paths_stat[i]) + 1,
+		CHECK(bss->called_close && bss->rets_close[i] != strlen(bss->paths_close[i]) + 1,
 		      "check",
 		      "failed to match stat return [%d]: %d vs %zd [%s]\n",
 		      i, bss->rets_close[i], strlen(bss->paths_close[i]) + 1,
-		      bss->paths_stat[i]);
+		      bss->paths_close[i]);
 	}
 
 cleanup:
diff --git a/tools/testing/selftests/bpf/progs/test_d_path.c b/tools/testing/selftests/bpf/progs/test_d_path.c
index 61f007855649..9e7223b4a555 100644
--- a/tools/testing/selftests/bpf/progs/test_d_path.c
+++ b/tools/testing/selftests/bpf/progs/test_d_path.c
@@ -15,6 +15,9 @@ char paths_close[MAX_FILES][MAX_PATH_LEN] = {};
 int rets_stat[MAX_FILES] = {};
 int rets_close[MAX_FILES] = {};
 
+int called_stat = 0;
+int called_close = 0;
+
 SEC("fentry/vfs_getattr")
 int BPF_PROG(prog_stat, struct path *path, struct kstat *stat,
 	     __u32 request_mask, unsigned int query_flags)
@@ -23,6 +26,8 @@ int BPF_PROG(prog_stat, struct path *path, struct kstat *stat,
 	__u32 cnt = cnt_stat;
 	int ret;
 
+	called_stat = 1;
+
 	if (pid != my_pid)
 		return 0;
 
@@ -42,6 +47,8 @@ int BPF_PROG(prog_close, struct file *file, void *id)
 	__u32 cnt = cnt_close;
 	int ret;
 
+	called_close = 1;
+
 	if (pid != my_pid)
 		return 0;
 
-- 
2.26.2

Re: [PATCH bpf-next] selftests/bpf: Check trampoline execution in d_path test

[Andrii Nakryiko]

On Thu, Sep 10, 2020 at 5:25 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.
>
> Adding flags that indicate trampolines were called and failing
> the test if neither of them got called.
>
>   $ sudo ./test_progs -t d_path
>   test_d_path:PASS:setup 0 nsec
>   ...
>   trigger_fstat_events:PASS:trigger 0 nsec
>   test_d_path:FAIL:124 trampolines not called
>   #22 d_path:FAIL
>   Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED
>
> If only one trampoline is called, it's still enough to test
> the helper, so only warn about missing trampoline call and
> continue in test.
>
>   $ sudo ./test_progs -t d_path -v
>   test_d_path:PASS:setup 0 nsec
>   ...
>   trigger_fstat_events:PASS:trigger 0 nsec
>   fentry/vfs_getattr not called
>   #22 d_path:OK
>   Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---

Acked-by: Andrii Nakryiko <andriin@fb.com>

>  .../testing/selftests/bpf/prog_tests/d_path.c | 25 +++++++++++++++----
>  .../testing/selftests/bpf/progs/test_d_path.c |  7 ++++++
>  2 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
> index fc12e0d445ff..ec15f7d1dd0a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/d_path.c
> +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
> @@ -120,26 +120,41 @@ void test_d_path(void)
>         if (err < 0)
>                 goto cleanup;
>
> +       if (!bss->called_stat && !bss->called_close) {
> +               PRINT_FAIL("trampolines not called\n");
> +               goto cleanup;
> +       }
> +
> +       if (!bss->called_stat) {
> +               fprintf(stdout, "fentry/vfs_getattr not called\n");
> +               goto cleanup;
> +       }
> +
> +       if (!bss->called_close) {
> +               fprintf(stdout, "fentry/filp_close not called\n");
> +               goto cleanup;
> +       }

not sure why you didn't go with `if (CHECK(!bss->called_close, ...`
for these checks, would even save you some typing.

> +
>         for (int i = 0; i < MAX_FILES; i++) {
> -               CHECK(strncmp(src.paths[i], bss->paths_stat[i], MAX_PATH_LEN),
> +               CHECK(bss->called_stat && strncmp(src.paths[i], bss->paths_stat[i], MAX_PATH_LEN),
>                       "check",
>                       "failed to get stat path[%d]: %s vs %s\n",
>                       i, src.paths[i], bss->paths_stat[i]);
> -               CHECK(strncmp(src.paths[i], bss->paths_close[i], MAX_PATH_LEN),
> +               CHECK(bss->called_close && strncmp(src.paths[i], bss->paths_close[i], MAX_PATH_LEN),
>                       "check",
>                       "failed to get close path[%d]: %s vs %s\n",
>                       i, src.paths[i], bss->paths_close[i]);
>                 /* The d_path helper returns size plus NUL char, hence + 1 */
> -               CHECK(bss->rets_stat[i] != strlen(bss->paths_stat[i]) + 1,
> +               CHECK(bss->called_stat && bss->rets_stat[i] != strlen(bss->paths_stat[i]) + 1,
>                       "check",
>                       "failed to match stat return [%d]: %d vs %zd [%s]\n",
>                       i, bss->rets_stat[i], strlen(bss->paths_stat[i]) + 1,
>                       bss->paths_stat[i]);
> -               CHECK(bss->rets_close[i] != strlen(bss->paths_stat[i]) + 1,
> +               CHECK(bss->called_close && bss->rets_close[i] != strlen(bss->paths_close[i]) + 1,
>                       "check",
>                       "failed to match stat return [%d]: %d vs %zd [%s]\n",
>                       i, bss->rets_close[i], strlen(bss->paths_close[i]) + 1,
> -                     bss->paths_stat[i]);
> +                     bss->paths_close[i]);


those `bss->called_xxx` guard conditions are a bit lost on reading, if
you reordered CHECKs, you could be more explicit:

if (bss->called_stat) {
    CHECK(...);
    CHECK(...);
}
if (bss->called_close) { ... }

>         }
>
>  cleanup:
> diff --git a/tools/testing/selftests/bpf/progs/test_d_path.c b/tools/testing/selftests/bpf/progs/test_d_path.c
> index 61f007855649..9e7223b4a555 100644
> --- a/tools/testing/selftests/bpf/progs/test_d_path.c
> +++ b/tools/testing/selftests/bpf/progs/test_d_path.c
> @@ -15,6 +15,9 @@ char paths_close[MAX_FILES][MAX_PATH_LEN] = {};
>  int rets_stat[MAX_FILES] = {};
>  int rets_close[MAX_FILES] = {};
>
> +int called_stat = 0;
> +int called_close = 0;
> +
>  SEC("fentry/vfs_getattr")
>  int BPF_PROG(prog_stat, struct path *path, struct kstat *stat,
>              __u32 request_mask, unsigned int query_flags)
> @@ -23,6 +26,8 @@ int BPF_PROG(prog_stat, struct path *path, struct kstat *stat,
>         __u32 cnt = cnt_stat;
>         int ret;
>
> +       called_stat = 1;
> +
>         if (pid != my_pid)
>                 return 0;
>
> @@ -42,6 +47,8 @@ int BPF_PROG(prog_close, struct file *file, void *id)
>         __u32 cnt = cnt_close;
>         int ret;
>
> +       called_close = 1;
> +
>         if (pid != my_pid)
>                 return 0;
>
> --
> 2.26.2
>

Re: [PATCH bpf-next] selftests/bpf: Check trampoline execution in d_path test

[Jiri Olsa]

On Thu, Sep 10, 2020 at 03:22:10PM -0700, Andrii Nakryiko wrote:
> On Thu, Sep 10, 2020 at 5:25 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.
> >
> > Adding flags that indicate trampolines were called and failing
> > the test if neither of them got called.
> >
> >   $ sudo ./test_progs -t d_path
> >   test_d_path:PASS:setup 0 nsec
> >   ...
> >   trigger_fstat_events:PASS:trigger 0 nsec
> >   test_d_path:FAIL:124 trampolines not called
> >   #22 d_path:FAIL
> >   Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED
> >
> > If only one trampoline is called, it's still enough to test
> > the helper, so only warn about missing trampoline call and
> > continue in test.
> >
> >   $ sudo ./test_progs -t d_path -v
> >   test_d_path:PASS:setup 0 nsec
> >   ...
> >   trigger_fstat_events:PASS:trigger 0 nsec
> >   fentry/vfs_getattr not called
> >   #22 d_path:OK
> >   Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
> >
> > Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> > ---
> 
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> 
> >  .../testing/selftests/bpf/prog_tests/d_path.c | 25 +++++++++++++++----
> >  .../testing/selftests/bpf/progs/test_d_path.c |  7 ++++++
> >  2 files changed, 27 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
> > index fc12e0d445ff..ec15f7d1dd0a 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/d_path.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
> > @@ -120,26 +120,41 @@ void test_d_path(void)
> >         if (err < 0)
> >                 goto cleanup;
> >
> > +       if (!bss->called_stat && !bss->called_close) {
> > +               PRINT_FAIL("trampolines not called\n");
> > +               goto cleanup;
> > +       }
> > +
> > +       if (!bss->called_stat) {
> > +               fprintf(stdout, "fentry/vfs_getattr not called\n");
> > +               goto cleanup;
> > +       }
> > +
> > +       if (!bss->called_close) {
> > +               fprintf(stdout, "fentry/filp_close not called\n");
> > +               goto cleanup;
> > +       }
> 
> not sure why you didn't go with `if (CHECK(!bss->called_close, ...`
> for these checks, would even save you some typing.

ok

> 
> > +
> >         for (int i = 0; i < MAX_FILES; i++) {
> > -               CHECK(strncmp(src.paths[i], bss->paths_stat[i], MAX_PATH_LEN),
> > +               CHECK(bss->called_stat && strncmp(src.paths[i], bss->paths_stat[i], MAX_PATH_LEN),
> >                       "check",
> >                       "failed to get stat path[%d]: %s vs %s\n",
> >                       i, src.paths[i], bss->paths_stat[i]);
> > -               CHECK(strncmp(src.paths[i], bss->paths_close[i], MAX_PATH_LEN),
> > +               CHECK(bss->called_close && strncmp(src.paths[i], bss->paths_close[i], MAX_PATH_LEN),
> >                       "check",
> >                       "failed to get close path[%d]: %s vs %s\n",
> >                       i, src.paths[i], bss->paths_close[i]);
> >                 /* The d_path helper returns size plus NUL char, hence + 1 */
> > -               CHECK(bss->rets_stat[i] != strlen(bss->paths_stat[i]) + 1,
> > +               CHECK(bss->called_stat && bss->rets_stat[i] != strlen(bss->paths_stat[i]) + 1,
> >                       "check",
> >                       "failed to match stat return [%d]: %d vs %zd [%s]\n",
> >                       i, bss->rets_stat[i], strlen(bss->paths_stat[i]) + 1,
> >                       bss->paths_stat[i]);
> > -               CHECK(bss->rets_close[i] != strlen(bss->paths_stat[i]) + 1,
> > +               CHECK(bss->called_close && bss->rets_close[i] != strlen(bss->paths_close[i]) + 1,
> >                       "check",
> >                       "failed to match stat return [%d]: %d vs %zd [%s]\n",
> >                       i, bss->rets_close[i], strlen(bss->paths_close[i]) + 1,
> > -                     bss->paths_stat[i]);
> > +                     bss->paths_close[i]);
> 
> 
> those `bss->called_xxx` guard conditions are a bit lost on reading, if
> you reordered CHECKs, you could be more explicit:
> 
> if (bss->called_stat) {
>     CHECK(...);
>     CHECK(...);
> }
> if (bss->called_close) { ... }

ok, will change

thanks,
jirka

Re: [PATCH bpf-next] selftests/bpf: Check trampoline execution in d_path test

[Alexei Starovoitov]

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.

Re: [PATCH bpf-next] selftests/bpf: Check trampoline execution in d_path test

[Jiri Olsa]

On Thu, Sep 10, 2020 at 05:46:21PM -0700, Alexei Starovoitov wrote:
> 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.

in my case vfs_getattr got inlined in vfs_statx_fd and both
of them are defined in fs/stat.c 

so the idea is that inlining will not happen if the function
is defined in another object? or less likely..?

we should be safe when it's called from module

> 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.

hm, what's the difference between vfs_getattr and security_inode_getattr
in this regard? I'd expect compiler could inline it same way as for vfs_getattr

jirka

Re: [PATCH bpf-next] selftests/bpf: Check trampoline execution in d_path test

[Alexei Starovoitov]

On Fri, Sep 11, 2020 at 6:16 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Thu, Sep 10, 2020 at 05:46:21PM -0700, Alexei Starovoitov wrote:
> > 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.
>
> in my case vfs_getattr got inlined in vfs_statx_fd and both
> of them are defined in fs/stat.c
>
> so the idea is that inlining will not happen if the function
> is defined in another object? or less likely..?

when it's in a different .o file. yes.
Very few folks build LTO kernels, so I propose to cross that bridge when
we get there.
Eventually we can replace security_inode_getattr
with bpf_lsm_inode_getattr or simply add noinline to security_inode_getattr.

> we should be safe when it's called from module

what do you mean?

> > 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.
>
> hm, what's the difference between vfs_getattr and security_inode_getattr
> in this regard? I'd expect compiler could inline it same way as for vfs_getattr

not really because they're in different files and LTO is not on.
Even with LTO the chances of inlining are small. The compiler will
consider profitability of it. Since there is a loop inside, it's unlikely.

Re: [PATCH bpf-next] selftests/bpf: Check trampoline execution in d_path test

[Jiri Olsa]

On Mon, Sep 14, 2020 at 07:30:33PM -0700, Alexei Starovoitov wrote:
> On Fri, Sep 11, 2020 at 6:16 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Thu, Sep 10, 2020 at 05:46:21PM -0700, Alexei Starovoitov wrote:
> > > 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.
> >
> > in my case vfs_getattr got inlined in vfs_statx_fd and both
> > of them are defined in fs/stat.c
> >
> > so the idea is that inlining will not happen if the function
> > is defined in another object? or less likely..?
> 
> when it's in a different .o file. yes.
> Very few folks build LTO kernels, so I propose to cross that bridge when
> we get there.
> Eventually we can replace security_inode_getattr
> with bpf_lsm_inode_getattr or simply add noinline to security_inode_getattr.
> 
> > we should be safe when it's called from module
> 
> what do you mean?

it's external call, so it will not get inlined

> 
> > > 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.
> >
> > hm, what's the difference between vfs_getattr and security_inode_getattr
> > in this regard? I'd expect compiler could inline it same way as for vfs_getattr
> 
> not really because they're in different files and LTO is not on.
> Even with LTO the chances of inlining are small. The compiler will
> consider profitability of it. Since there is a loop inside, it's unlikely.

ok, thanks for info

I'll use that security_inode_getattr instead of vfs_getattr

jirka