[LTP] [PATCH] fanotify14: Test disallow sb/mount mark on anonymous pipe

[Amir Goldstein]

On Mon, Jul 10, 2023 at 6:50 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Amir,
>
> > This case was retroactively disallowed.
>
> > This test is meant to encourage the backporting of commit 69562eb0bd3e
> > ("fanotify: disallow mount/sb marks on kernel internal pseudo fs") to
> > all stable kernels.
>
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
>
> > Petr,
>
> > This tests for a behavior that we consider broken since the dawn of
> > fanotify.
>
> > The fix was merged to v6.5-rc1.
> > I've already posted backport patches for kernels > v5.0.
> > I am not planning to post backport patches for older kernels.
>
> I see
> https://lore.kernel.org/stable/20230710133205.1154168-1-amir73il@gmail.com/
>
> I'll suggest to wait till Greg releases the backport (should be quick enough).
>

ok.

> > Even though the two new test cases do not use FAN_REPORT_FID,
> > fanotify14 requires FAN_REPORT_FID, so it is not going to run these
> > test cases on kernel < v5.1 anyway.
>
> > Thanks,
> > Amir.
>
> >  .../kernel/syscalls/fanotify/fanotify14.c     | 32 +++++++++++++++++--
> >  1 file changed, 30 insertions(+), 2 deletions(-)
>
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
> > index bfa0349fe..063a9f96f 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify14.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
> > @@ -19,6 +19,9 @@
> >   *
> >   *     ceaf69f8eadc fanotify: do not allow setting dirent events in mask of non-dir
> >   *     8698e3bab4dd fanotify: refine the validation checks on non-dir inode mask
> > + *
> > + * The pipes test cases are regression tests for commit:
> > + *     69562eb0bd3e fanotify: disallow mount/sb marks on kernel internal pseudo fs
> >   */
>
> >  #define _GNU_SOURCE
> > @@ -40,6 +43,7 @@
>
> >  #define FLAGS_DESC(flags) {(flags), (#flags)}
>
> > +static int pipes[2] = {-1, -1};
> >  static int fanotify_fd;
> >  static int fan_report_target_fid_unsupported;
> >  static int ignore_mark_unsupported;
> > @@ -60,6 +64,7 @@ static struct test_case_t {
> >       /* when mask.flags == 0, fanotify_init() is expected to fail */
> >       struct test_case_flags_t mask;
> >       int expected_errno;
> > +     int *pfd;
>
> This produces warnings:
> fanotify14.c:70:9: warning: missing initializer for field ‘pfd’ of ‘struct test_case_t’ [-Wmissing-field-initializers]
>    70 |         {FLAGS_DESC(FAN_CLASS_CONTENT | FAN_REPORT_FID), {}, {}, EINVAL},
>       |         ^
> fanotify14.c:67:14: note: ‘pfd’ declared here
>    67 |         int *pfd;
>       |              ^~~
> fanotify14.c:73:9: warning: missing initializer for field ‘pfd’ of ‘struct test_case_t’ [-Wmissing-field-initializers]
>    73 |         {FLAGS_DESC(FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID), {}, {}, EINVAL},
>       |         ^
> fanotify14.c:67:14: note: ‘pfd’ declared here
>    67 |         int *pfd;
>       |              ^~~
>
> Could you please fix them? I guess pfd must be NULL when unused.
>

ok. but I have to ask,
what is the value of explicitly initializing all the old test cases to
pfd = NULL?
and what is wrong with default NULL initializers?
Is it a deliberate decision of LTP to care about this warning?
it's a classic pattern for what this patch does -
add a new field to test case which all the existing test cases
should not care about.

Also, I have always seen these warnings for struct tst_test.

fanotify14.c:284:1: warning: missing initializer for field
'needs_cmds' of 'struct tst_test' [-Wmissing-field-initializers]
  284 | };
      | ^
In file included from fanotify14.c:28:
../../../../include/tst_test.h:324:21: note: 'needs_cmds' declared here
  324 |  const char *const *needs_cmds;
      |                     ^~~~~~~~~~

Must we really initialize an empty needs_cmds array for every test?
Seems pointless to me, but I may not have the bigger picture.

Thanks,
Amir.