[LTP] [RFC 2/3] syscalls/fanotify03: included execve() to generate_events() to increase test coverage

[Matthew Bobrowski]

On Wed, Oct 24, 2018 at 08:31:22AM +0300, Amir Goldstein wrote:
> On Wed, Oct 24, 2018 at 6:28 AM Matthew Bobrowski
> <mbobrowski@mbobrowski.org> wrote:
> >
> > * Created an executable helper program 'fanotify_child' so that can be
> >   used within fanotify tests
> >
> > * Defined .resource_files so that additional test resources can be
> >   copied across to the tmp working directory i.e. fanotify_child
> >
> > * Updated generate_events() so that it now includes a call to execve()
> >   on fanotify_child. This is so that we can increase the overall test
> >   coverage by generating more events on a watched object
> >
> > * Updated each tcase events[] to accommodate for the additional events
> >   generated by execve()
> >
> > Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> > ---
> >  testcases/kernel/syscalls/fanotify/.gitignore |  1 +
> >  .../kernel/syscalls/fanotify/fanotify03.c     | 83 ++++++++++++-------
> >  .../kernel/syscalls/fanotify/fanotify_child.c | 14 ++++
> >  3 files changed, 66 insertions(+), 32 deletions(-)
> >  create mode 100644 testcases/kernel/syscalls/fanotify/fanotify_child.c
> >
> > diff --git a/testcases/kernel/syscalls/fanotify/.gitignore b/testcases/kernel/syscalls/fanotify/.gitignore
> > index c26f2bd27..af420b8b3 100644
> > --- a/testcases/kernel/syscalls/fanotify/.gitignore
> > +++ b/testcases/kernel/syscalls/fanotify/.gitignore
> > @@ -8,3 +8,4 @@
> >  /fanotify08
> >  /fanotify09
> >  /fanotify10
> > +/fanotify_child
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
> > index cca15aa00..f9418ee6b 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify03.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
> > @@ -35,6 +35,7 @@
> >
> >  #define BUF_SIZE 256
> >  #define TST_TOTAL 3
> > +#define TEST_APP "fanotify_child"
> >
> >  static char fname[BUF_SIZE];
> >  static char buf[BUF_SIZE];
> > @@ -60,28 +61,31 @@ static struct tcase {
> >         {
> >                 "inode mark permission events",
> >                 INIT_FANOTIFY_MARK_TYPE(INODE),
> > -               FAN_OPEN_PERM | FAN_ACCESS_PERM, 2,
> > +               FAN_OPEN_PERM | FAN_ACCESS_PERM, 3,
> >                 {
> >                         {FAN_OPEN_PERM, FAN_ALLOW},
> > -                       {FAN_ACCESS_PERM, FAN_DENY}
> > +                       {FAN_ACCESS_PERM, FAN_DENY},
> > +                       {FAN_OPEN_PERM, FAN_DENY}
> >                 }
> >         },
> >         {
> >                 "mount mark permission events",
> >                 INIT_FANOTIFY_MARK_TYPE(MOUNT),
> > -               FAN_OPEN_PERM | FAN_ACCESS_PERM, 2,
> > +               FAN_OPEN_PERM | FAN_ACCESS_PERM, 3,
> >                 {
> >                         {FAN_OPEN_PERM, FAN_ALLOW},
> > -                       {FAN_ACCESS_PERM, FAN_DENY}
> > +                       {FAN_ACCESS_PERM, FAN_DENY},
> > +                       {FAN_OPEN_PERM, FAN_DENY}
> >                 }
> >         },
> >         {
> >                 "filesystem mark permission events",
> >                 INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> > -               FAN_OPEN_PERM | FAN_ACCESS_PERM, 2,
> > +               FAN_OPEN_PERM | FAN_ACCESS_PERM, 3,
> >                 {
> >                         {FAN_OPEN_PERM, FAN_ALLOW},
> > -                       {FAN_ACCESS_PERM, FAN_DENY}
> > +                       {FAN_ACCESS_PERM, FAN_DENY},
> > +                       {FAN_OPEN_PERM, FAN_DENY}
> >                 }
> >         }
> >  };
> > @@ -89,9 +93,10 @@ static struct tcase {
> >  static void generate_events(void)
> >  {
> >         int fd;
> > +       char *const argv[] = {TEST_APP, NULL};
> >
> >         /*
> > -        * generate sequence of events
> > +        * Generate sequence of events
> >          */
> >         if ((fd = open(fname, O_RDWR | O_CREAT, 0700)) == -1)
> >                 exit(1);
> > @@ -104,6 +109,9 @@ static void generate_events(void)
> >
> >         if (close(fd) == -1)
> >                 exit(4);
> > +
> > +       if (execve(TEST_APP, argv, environ) != -1)
> > +               exit(5);
> >  }
> >
> 
> I am a bit puzzled.
> Did you test this point in the series?
> 
Yes, I did test it at this point.

> If child is denied read access then it exits before execve(), so you
> shouldn't be adding an extra OPEN event in event_set of exiting
> test cases, which DENY ACCESS??
> 
I don't know whether I'm understanding you correctly, but I do believe
that it's needed based on the following facts. Within generate_events()
the system logic flow is as follows:

open(...)	== -1
write(...)	== -1	
read(...)	!= -1
close(...)	== -1
execve(...)	== -1

Based on the sequence of system calls and comparison operators above, then
if read access is denied (FAN_DENY) then it does not exit, as that is what
is expected, so it falls through to the next set system calls. Based on
that fact, an additional event {FAN_OPEN_PERM, FAN_DENY} is needed as this
is the first event that is generated when the mask FAN_OPEN_EXEC_PERM
hasn't been provided as a mask.

-- 
Matthew Bobrowski <mbobrowski@mbobrowski.org>