From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matthew Bobrowski Date: Thu, 25 Oct 2018 17:39:49 +1100 Subject: [LTP] [RFC 2/3] syscalls/fanotify03: included execve() to generate_events() to increase test coverage In-Reply-To: References: <2b2a0e71fd63b7536f0cdfbbc24570283ea98c33.1540348505.git.mbobrowski@mbobrowski.org> Message-ID: <20181025063924.GA7235@development.internal.lab> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ltp@lists.linux.it On Wed, Oct 24, 2018 at 08:31:22AM +0300, Amir Goldstein wrote: > On Wed, Oct 24, 2018 at 6:28 AM Matthew Bobrowski > 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 > > --- > > 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