* [LTP] [RFC 1/3] syscalls/fanotify03: defined additional tcase members to support more tcase control
2018-10-24 3:26 [LTP] [RFC 0/3] syscalls/fanotify03: add support and tests for new FAN_OPEN_EXEC_PERM flag Matthew Bobrowski
@ 2018-10-24 3:27 ` Matthew Bobrowski
2018-10-24 5:23 ` Amir Goldstein
2018-10-24 3:27 ` [LTP] [RFC 2/3] syscalls/fanotify03: included execve() to generate_events() to increase test coverage Matthew Bobrowski
2018-10-24 3:28 ` [LTP] [RFC 3/3] syscalls/fanotify03: add FAN_OPEN_EXEC_PERM tcase support Matthew Bobrowski
2 siblings, 1 reply; 11+ messages in thread
From: Matthew Bobrowski @ 2018-10-24 3:27 UTC (permalink / raw)
To: ltp
* Included the event mask member to be used for fanotify_mark() within
the tcase. This allows control over what event types are to be
generated on a tcase by tcase level as oppose to having it globally
defined for all tcases.
* Included the event_set and event_resp arrays into a tcase so that
the sequence of expected events can be defined on a test case by test
case level. Both event_set and event_resp are now represented by the
event struct.
* Added event_count member to tcase in order to represent the number of
expected events for each tcase.
* Fanotify permission events cannot be merged, thus cleaned up some
unnecessary code in test_fanotify() that checks whether any additional
bits are left within event->mask.
Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
---
.../kernel/syscalls/fanotify/fanotify03.c | 90 ++++++++++---------
1 file changed, 50 insertions(+), 40 deletions(-)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
index 5c105ed32..cca15aa00 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify03.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
@@ -42,28 +42,48 @@ static volatile int fd_notify;
static pid_t child_pid;
-static unsigned long long event_set[EVENT_MAX];
-static unsigned int event_resp[EVENT_MAX];
-
static char event_buf[EVENT_BUF_LEN];
static int support_perm_events;
+struct event {
+ unsigned long long type;
+ unsigned int response;
+};
+
static struct tcase {
const char *tname;
struct fanotify_mark_type mark;
+ unsigned long long mask;
+ int event_count;
+ struct event event_set[EVENT_MAX];
} tcases[] = {
{
"inode mark permission events",
INIT_FANOTIFY_MARK_TYPE(INODE),
+ FAN_OPEN_PERM | FAN_ACCESS_PERM, 2,
+ {
+ {FAN_OPEN_PERM, FAN_ALLOW},
+ {FAN_ACCESS_PERM, FAN_DENY}
+ }
},
{
"mount mark permission events",
INIT_FANOTIFY_MARK_TYPE(MOUNT),
+ FAN_OPEN_PERM | FAN_ACCESS_PERM, 2,
+ {
+ {FAN_OPEN_PERM, FAN_ALLOW},
+ {FAN_ACCESS_PERM, FAN_DENY}
+ }
},
{
"filesystem mark permission events",
INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
- },
+ FAN_OPEN_PERM | FAN_ACCESS_PERM, 2,
+ {
+ {FAN_OPEN_PERM, FAN_ALLOW},
+ {FAN_ACCESS_PERM, FAN_DENY}
+ }
+ }
};
static void generate_events(void)
@@ -146,8 +166,7 @@ static int setup_mark(unsigned int n)
fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
- if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
- FAN_ACCESS_PERM | FAN_OPEN_PERM,
+ if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
AT_FDCWD, fname) < 0) {
if (errno == EINVAL && support_perm_events &&
mark->flag == FAN_MARK_FILESYSTEM) {
@@ -167,7 +186,7 @@ static int setup_mark(unsigned int n)
}
} else {
/*
- * To distigouish between perm event not supported and
+ * To distinguish between perm event not supported and
* filesystem mark not supported.
*/
support_perm_events = 1;
@@ -179,32 +198,22 @@ static int setup_mark(unsigned int n)
static void test_fanotify(unsigned int n)
{
- int tst_count;
int ret, len = 0, i = 0, test_num = 0;
+ struct tcase *tc = &tcases[n];
+ struct event *event_set = tc->event_set;
if (setup_mark(n) != 0)
return;
run_child();
- tst_count = 0;
-
- event_set[tst_count] = FAN_OPEN_PERM;
- event_resp[tst_count++] = FAN_ALLOW;
- event_set[tst_count] = FAN_ACCESS_PERM;
- event_resp[tst_count++] = FAN_DENY;
-
- /* tst_count + 1 is for checking child return value */
- if (TST_TOTAL != tst_count + 1) {
- tst_brk(TBROK,
- "TST_TOTAL and tst_count do not match");
- }
- tst_count = 0;
-
/*
- * check events
+ * Process events
+ *
+ * tc->count + 1 is to accommodate for checking the child process
+ * return value
*/
- while (test_num < TST_TOTAL && fd_notify != -1) {
+ while (test_num < tc->event_count + 1 && fd_notify != -1) {
struct fanotify_event_metadata *event;
if (i == len) {
@@ -222,12 +231,12 @@ static void test_fanotify(unsigned int n)
}
event = (struct fanotify_event_metadata *)&event_buf[i];
- if (!(event->mask & event_set[test_num])) {
+ if (!(event->mask & event_set[test_num].type)) {
tst_res(TFAIL,
"got event: mask=%llx (expected %llx) "
"pid=%u fd=%d",
(unsigned long long)event->mask,
- event_set[test_num],
+ event_set[test_num].type,
(unsigned)event->pid, event->fd);
} else if (event->pid != child_pid) {
tst_res(TFAIL,
@@ -243,29 +252,30 @@ static void test_fanotify(unsigned int n)
(unsigned long long)event->mask,
(unsigned)event->pid, event->fd);
}
- /* Write response to permission event */
- if (event_set[test_num] & FAN_ALL_PERM_EVENTS) {
+
+ /* Write response to the permission event */
+ if (event_set[test_num].type & FAN_ALL_PERM_EVENTS) {
struct fanotify_response resp;
resp.fd = event->fd;
- resp.response = event_resp[test_num];
- SAFE_WRITE(1, fd_notify, &resp,
- sizeof(resp));
- }
- event->mask &= ~event_set[test_num];
- /* No events left in current mask? Go for next event */
- if (event->mask == 0) {
- i += event->event_len;
- if (event->fd != FAN_NOFD)
- SAFE_CLOSE(event->fd);
+ resp.response = event_set[test_num].response;
+ SAFE_WRITE(1, fd_notify, &resp, sizeof(resp));
}
+
+ i += event->event_len;
+
+ if (event->fd != FAN_NOFD)
+ SAFE_CLOSE(event->fd);
+
test_num++;
}
- for (; test_num < TST_TOTAL - 1; test_num++) {
+
+ for (; test_num < tc->event_count; test_num++) {
tst_res(TFAIL, "didn't get event: mask=%llx",
- event_set[test_num]);
+ event_set[test_num].type);
}
+
check_child();
if (fd_notify > 0)
--
2.17.2
--
Matthew Bobrowski <mbobrowski@mbobrowski.org>
^ permalink raw reply related [flat|nested] 11+ messages in thread* [LTP] [RFC 1/3] syscalls/fanotify03: defined additional tcase members to support more tcase control
2018-10-24 3:27 ` [LTP] [RFC 1/3] syscalls/fanotify03: defined additional tcase members to support more tcase control Matthew Bobrowski
@ 2018-10-24 5:23 ` Amir Goldstein
2018-10-25 6:19 ` Matthew Bobrowski
0 siblings, 1 reply; 11+ messages in thread
From: Amir Goldstein @ 2018-10-24 5:23 UTC (permalink / raw)
To: ltp
On Wed, Oct 24, 2018 at 6:27 AM Matthew Bobrowski
<mbobrowski@mbobrowski.org> wrote:
>
> * Included the event mask member to be used for fanotify_mark() within
> the tcase. This allows control over what event types are to be
> generated on a tcase by tcase level as oppose to having it globally
> defined for all tcases.
>
> * Included the event_set and event_resp arrays into a tcase so that
> the sequence of expected events can be defined on a test case by test
> case level. Both event_set and event_resp are now represented by the
> event struct.
>
> * Added event_count member to tcase in order to represent the number of
> expected events for each tcase.
>
> * Fanotify permission events cannot be merged, thus cleaned up some
> unnecessary code in test_fanotify() that checks whether any additional
> bits are left within event->mask.
>
> Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> ---
Looks good! nits below.
> .../kernel/syscalls/fanotify/fanotify03.c | 90 ++++++++++---------
> 1 file changed, 50 insertions(+), 40 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
> index 5c105ed32..cca15aa00 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify03.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
> @@ -42,28 +42,48 @@ static volatile int fd_notify;
>
> static pid_t child_pid;
>
> -static unsigned long long event_set[EVENT_MAX];
> -static unsigned int event_resp[EVENT_MAX];
> -
> static char event_buf[EVENT_BUF_LEN];
> static int support_perm_events;
>
> +struct event {
> + unsigned long long type;
It's better to name this also 'mask' as in the near future you will
want to assign it the value of OPEN_PERM|OPEN_EXEC_PERM
which is not a 'type'.
> + unsigned int response;
> +};
> +
> static struct tcase {
> const char *tname;
> struct fanotify_mark_type mark;
> + unsigned long long mask;
> + int event_count;
> + struct event event_set[EVENT_MAX];
Better change the value of EVENT_MAX...
or even decouple it from EVENT_BUF_LEN
> } tcases[] = {
> {
> "inode mark permission events",
> INIT_FANOTIFY_MARK_TYPE(INODE),
> + FAN_OPEN_PERM | FAN_ACCESS_PERM, 2,
> + {
> + {FAN_OPEN_PERM, FAN_ALLOW},
> + {FAN_ACCESS_PERM, FAN_DENY}
> + }
> },
> {
> "mount mark permission events",
> INIT_FANOTIFY_MARK_TYPE(MOUNT),
> + FAN_OPEN_PERM | FAN_ACCESS_PERM, 2,
> + {
> + {FAN_OPEN_PERM, FAN_ALLOW},
> + {FAN_ACCESS_PERM, FAN_DENY}
> + }
> },
> {
> "filesystem mark permission events",
> INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> - },
> + FAN_OPEN_PERM | FAN_ACCESS_PERM, 2,
> + {
> + {FAN_OPEN_PERM, FAN_ALLOW},
> + {FAN_ACCESS_PERM, FAN_DENY}
> + }
> + }
> };
>
> static void generate_events(void)
> @@ -146,8 +166,7 @@ static int setup_mark(unsigned int n)
>
> fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
>
> - if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> - FAN_ACCESS_PERM | FAN_OPEN_PERM,
> + if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
> AT_FDCWD, fname) < 0) {
> if (errno == EINVAL && support_perm_events &&
> mark->flag == FAN_MARK_FILESYSTEM) {
> @@ -167,7 +186,7 @@ static int setup_mark(unsigned int n)
> }
> } else {
> /*
> - * To distigouish between perm event not supported and
> + * To distinguish between perm event not supported and
> * filesystem mark not supported.
> */
> support_perm_events = 1;
> @@ -179,32 +198,22 @@ static int setup_mark(unsigned int n)
>
> static void test_fanotify(unsigned int n)
> {
> - int tst_count;
> int ret, len = 0, i = 0, test_num = 0;
> + struct tcase *tc = &tcases[n];
> + struct event *event_set = tc->event_set;
>
> if (setup_mark(n) != 0)
> return;
>
> run_child();
>
> - tst_count = 0;
> -
> - event_set[tst_count] = FAN_OPEN_PERM;
> - event_resp[tst_count++] = FAN_ALLOW;
> - event_set[tst_count] = FAN_ACCESS_PERM;
> - event_resp[tst_count++] = FAN_DENY;
> -
> - /* tst_count + 1 is for checking child return value */
> - if (TST_TOTAL != tst_count + 1) {
> - tst_brk(TBROK,
> - "TST_TOTAL and tst_count do not match");
> - }
> - tst_count = 0;
> -
> /*
> - * check events
> + * Process events
> + *
> + * tc->count + 1 is to accommodate for checking the child process
> + * return value
> */
> - while (test_num < TST_TOTAL && fd_notify != -1) {
> + while (test_num < tc->event_count + 1 && fd_notify != -1) {
> struct fanotify_event_metadata *event;
>
> if (i == len) {
> @@ -222,12 +231,12 @@ static void test_fanotify(unsigned int n)
> }
>
> event = (struct fanotify_event_metadata *)&event_buf[i];
> - if (!(event->mask & event_set[test_num])) {
> + if (!(event->mask & event_set[test_num].type)) {
/* Permission events cannot be merged, so... */
+ if (event->mask != event_set[test_num].mask) {
Thanks,
Amir.
^ permalink raw reply [flat|nested] 11+ messages in thread* [LTP] [RFC 1/3] syscalls/fanotify03: defined additional tcase members to support more tcase control
2018-10-24 5:23 ` Amir Goldstein
@ 2018-10-25 6:19 ` Matthew Bobrowski
0 siblings, 0 replies; 11+ messages in thread
From: Matthew Bobrowski @ 2018-10-25 6:19 UTC (permalink / raw)
To: ltp
On Wed, Oct 24, 2018 at 08:23:47AM +0300, Amir Goldstein wrote:
> On Wed, Oct 24, 2018 at 6:27 AM Matthew Bobrowski
> <mbobrowski@mbobrowski.org> wrote:
> >
> > * Included the event mask member to be used for fanotify_mark() within
> > the tcase. This allows control over what event types are to be
> > generated on a tcase by tcase level as oppose to having it globally
> > defined for all tcases.
> >
> > * Included the event_set and event_resp arrays into a tcase so that
> > the sequence of expected events can be defined on a test case by test
> > case level. Both event_set and event_resp are now represented by the
> > event struct.
> >
> > * Added event_count member to tcase in order to represent the number of
> > expected events for each tcase.
> >
> > * Fanotify permission events cannot be merged, thus cleaned up some
> > unnecessary code in test_fanotify() that checks whether any additional
> > bits are left within event->mask.
> >
> > Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> > ---
>
> Looks good! nits below.
>
> > .../kernel/syscalls/fanotify/fanotify03.c | 90 ++++++++++---------
> > 1 file changed, 50 insertions(+), 40 deletions(-)
> >
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
> > index 5c105ed32..cca15aa00 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify03.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
> > @@ -42,28 +42,48 @@ static volatile int fd_notify;
> >
> > static pid_t child_pid;
> >
> > -static unsigned long long event_set[EVENT_MAX];
> > -static unsigned int event_resp[EVENT_MAX];
> > -
> > static char event_buf[EVENT_BUF_LEN];
> > static int support_perm_events;
> >
> > +struct event {
> > + unsigned long long type;
>
> It's better to name this also 'mask' as in the near future you will
> want to assign it the value of OPEN_PERM|OPEN_EXEC_PERM
> which is not a 'type'.
>
Yes, I also agree.
> > + unsigned int response;
> > +};
> > +
> > static struct tcase {
> > const char *tname;
> > struct fanotify_mark_type mark;
> > + unsigned long long mask;
> > + int event_count;
> > + struct event event_set[EVENT_MAX];
>
> Better change the value of EVENT_MAX...
> or even decouple it from EVENT_BUF_LEN
>
Good point. OK, providing that's the case, I think doing something along
the lines of the below would be reasonable? I'm under the assumption that
this is what you meant by changing the value?
#define EVENT_SET_SIZE (sizeof (struct event) * 16)
> > } tcases[] = {
> > {
> > "inode mark permission events",
> > INIT_FANOTIFY_MARK_TYPE(INODE),
> > + FAN_OPEN_PERM | FAN_ACCESS_PERM, 2,
> > + {
> > + {FAN_OPEN_PERM, FAN_ALLOW},
> > + {FAN_ACCESS_PERM, FAN_DENY}
> > + }
> > },
> > {
> > "mount mark permission events",
> > INIT_FANOTIFY_MARK_TYPE(MOUNT),
> > + FAN_OPEN_PERM | FAN_ACCESS_PERM, 2,
> > + {
> > + {FAN_OPEN_PERM, FAN_ALLOW},
> > + {FAN_ACCESS_PERM, FAN_DENY}
> > + }
> > },
> > {
> > "filesystem mark permission events",
> > INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> > - },
> > + FAN_OPEN_PERM | FAN_ACCESS_PERM, 2,
> > + {
> > + {FAN_OPEN_PERM, FAN_ALLOW},
> > + {FAN_ACCESS_PERM, FAN_DENY}
> > + }
> > + }
> > };
> >
> > static void generate_events(void)
> > @@ -146,8 +166,7 @@ static int setup_mark(unsigned int n)
> >
> > fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
> >
> > - if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> > - FAN_ACCESS_PERM | FAN_OPEN_PERM,
> > + if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
> > AT_FDCWD, fname) < 0) {
> > if (errno == EINVAL && support_perm_events &&
> > mark->flag == FAN_MARK_FILESYSTEM) {
> > @@ -167,7 +186,7 @@ static int setup_mark(unsigned int n)
> > }
> > } else {
> > /*
> > - * To distigouish between perm event not supported and
> > + * To distinguish between perm event not supported and
> > * filesystem mark not supported.
> > */
> > support_perm_events = 1;
> > @@ -179,32 +198,22 @@ static int setup_mark(unsigned int n)
> >
> > static void test_fanotify(unsigned int n)
> > {
> > - int tst_count;
> > int ret, len = 0, i = 0, test_num = 0;
> > + struct tcase *tc = &tcases[n];
> > + struct event *event_set = tc->event_set;
> >
> > if (setup_mark(n) != 0)
> > return;
> >
> > run_child();
> >
> > - tst_count = 0;
> > -
> > - event_set[tst_count] = FAN_OPEN_PERM;
> > - event_resp[tst_count++] = FAN_ALLOW;
> > - event_set[tst_count] = FAN_ACCESS_PERM;
> > - event_resp[tst_count++] = FAN_DENY;
> > -
> > - /* tst_count + 1 is for checking child return value */
> > - if (TST_TOTAL != tst_count + 1) {
> > - tst_brk(TBROK,
> > - "TST_TOTAL and tst_count do not match");
> > - }
> > - tst_count = 0;
> > -
> > /*
> > - * check events
> > + * Process events
> > + *
> > + * tc->count + 1 is to accommodate for checking the child process
> > + * return value
> > */
> > - while (test_num < TST_TOTAL && fd_notify != -1) {
> > + while (test_num < tc->event_count + 1 && fd_notify != -1) {
> > struct fanotify_event_metadata *event;
> >
> > if (i == len) {
> > @@ -222,12 +231,12 @@ static void test_fanotify(unsigned int n)
> > }
> >
> > event = (struct fanotify_event_metadata *)&event_buf[i];
> > - if (!(event->mask & event_set[test_num])) {
> > + if (!(event->mask & event_set[test_num].type)) {
>
> /* Permission events cannot be merged, so... */
> + if (event->mask != event_set[test_num].mask) {
>
Updated.
--
Matthew Bobrowski <mbobrowski@mbobrowski.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [RFC 2/3] syscalls/fanotify03: included execve() to generate_events() to increase test coverage
2018-10-24 3:26 [LTP] [RFC 0/3] syscalls/fanotify03: add support and tests for new FAN_OPEN_EXEC_PERM flag Matthew Bobrowski
2018-10-24 3:27 ` [LTP] [RFC 1/3] syscalls/fanotify03: defined additional tcase members to support more tcase control Matthew Bobrowski
@ 2018-10-24 3:27 ` Matthew Bobrowski
2018-10-24 5:31 ` Amir Goldstein
2018-10-24 3:28 ` [LTP] [RFC 3/3] syscalls/fanotify03: add FAN_OPEN_EXEC_PERM tcase support Matthew Bobrowski
2 siblings, 1 reply; 11+ messages in thread
From: Matthew Bobrowski @ 2018-10-24 3:27 UTC (permalink / raw)
To: ltp
* 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);
}
static void child_handler(int tmp)
@@ -131,6 +139,7 @@ static void run_child(void)
}
child_pid = SAFE_FORK();
+
if (child_pid == 0) {
/* Child will generate events now */
close(fd_notify);
@@ -161,38 +170,43 @@ static void check_child(void)
static int setup_mark(unsigned int n)
{
+ unsigned int i = 0;
struct tcase *tc = &tcases[n];
struct fanotify_mark_type *mark = &tc->mark;
+ char *const files[] = {fname, TEST_APP};
+ tst_res(TINFO, "Test #%d: %s", n, tc->tname);
fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
- if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
- AT_FDCWD, fname) < 0) {
- if (errno == EINVAL && support_perm_events &&
- mark->flag == FAN_MARK_FILESYSTEM) {
- tst_res(TCONF,
- "FAN_MARK_FILESYSTEM not supported in kernel?");
- return -1;
- } else if (errno == EINVAL) {
- tst_brk(TCONF | TERRNO,
- "CONFIG_FANOTIFY_ACCESS_PERMISSIONS not "
- "configured in kernel?");
+ for (; i < ARRAY_SIZE(files); i++) {
+ if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
+ tc->mask, AT_FDCWD, files[i]) < 0) {
+ if (errno == EINVAL && support_perm_events &&
+ mark->flag == FAN_MARK_FILESYSTEM) {
+ tst_res(TCONF,
+ "FAN_MARK_FILESYSTEM not supported in "
+ "kernel?");
+ return -1;
+ } else if (errno == EINVAL) {
+ tst_brk(TCONF | TERRNO,
+ "CONFIG_FANOTIFY_ACCESS_PERMISSIONS "
+ "not configured in kernel?");
+ } else {
+ tst_brk(TBROK | TERRNO,
+ "fanotify_mark (%d, FAN_MARK_ADD | %s, "
+ "FAN_ACCESS_PERM | FAN_OPEN_PERM, "
+ "AT_FDCWD, %s) failed.",
+ fd_notify, mark->name, fname);
+ }
} else {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_ADD | %s, "
- "FAN_ACCESS_PERM | FAN_OPEN_PERM, "
- "AT_FDCWD, %s) failed.",
- fd_notify, mark->name, fname);
+ /*
+ * To distinguish between perm event not supported and
+ * filesystem mark not supported.
+ */
+ support_perm_events = 1;
}
- } else {
- /*
- * To distinguish between perm event not supported and
- * filesystem mark not supported.
- */
- support_perm_events = 1;
}
- tst_res(TINFO, "Test #%d: %s", n, tc->tname);
return 0;
}
@@ -294,14 +308,19 @@ static void cleanup(void)
SAFE_CLOSE(fd_notify);
}
+static const char *const resource_files[] = {
+ TEST_APP,
+ NULL
+};
+
static struct tst_test test = {
.test = test_fanotify,
.tcnt = ARRAY_SIZE(tcases),
.setup = setup,
.cleanup = cleanup,
- .needs_tmpdir = 1,
.forks_child = 1,
- .needs_root = 1
+ .needs_root = 1,
+ .resource_files = resource_files
};
#else
diff --git a/testcases/kernel/syscalls/fanotify/fanotify_child.c b/testcases/kernel/syscalls/fanotify/fanotify_child.c
new file mode 100644
index 000000000..f43068264
--- /dev/null
+++ b/testcases/kernel/syscalls/fanotify/fanotify_child.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 Matthew Bobrowski. All Rights Reserved.
+ *
+ * Started by Matthew Bobrowski <mbobrowski@mbobrowski.org>
+ *
+ * DESCRIPTION
+ * Simple helper program that can be simply invoked from fanotify tests
+ */
+
+int main(void)
+{
+ return 0;
+}
--
2.17.2
--
Matthew Bobrowski <mbobrowski@mbobrowski.org>
^ permalink raw reply related [flat|nested] 11+ messages in thread* [LTP] [RFC 2/3] syscalls/fanotify03: included execve() to generate_events() to increase test coverage
2018-10-24 3:27 ` [LTP] [RFC 2/3] syscalls/fanotify03: included execve() to generate_events() to increase test coverage Matthew Bobrowski
@ 2018-10-24 5:31 ` Amir Goldstein
2018-10-25 6:39 ` Matthew Bobrowski
0 siblings, 1 reply; 11+ messages in thread
From: Amir Goldstein @ 2018-10-24 5:31 UTC (permalink / raw)
To: ltp
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?
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??
Thanks,
Amir.
^ permalink raw reply [flat|nested] 11+ messages in thread* [LTP] [RFC 2/3] syscalls/fanotify03: included execve() to generate_events() to increase test coverage
2018-10-24 5:31 ` Amir Goldstein
@ 2018-10-25 6:39 ` Matthew Bobrowski
2018-10-25 6:46 ` Amir Goldstein
0 siblings, 1 reply; 11+ messages in thread
From: Matthew Bobrowski @ 2018-10-25 6:39 UTC (permalink / raw)
To: ltp
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>
^ permalink raw reply [flat|nested] 11+ messages in thread* [LTP] [RFC 2/3] syscalls/fanotify03: included execve() to generate_events() to increase test coverage
2018-10-25 6:39 ` Matthew Bobrowski
@ 2018-10-25 6:46 ` Amir Goldstein
0 siblings, 0 replies; 11+ messages in thread
From: Amir Goldstein @ 2018-10-25 6:46 UTC (permalink / raw)
To: ltp
On Thu, Oct 25, 2018 at 9:39 AM Matthew Bobrowski
<mbobrowski@mbobrowski.org> wrote:
>
> 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.
>
Right. I missed the fact that read failure is expected in generate_events
You really managed to extend the test coverage with minimal changes
to the test ;-)
Thanks,
Amir.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [RFC 3/3] syscalls/fanotify03: add FAN_OPEN_EXEC_PERM tcase support
2018-10-24 3:26 [LTP] [RFC 0/3] syscalls/fanotify03: add support and tests for new FAN_OPEN_EXEC_PERM flag Matthew Bobrowski
2018-10-24 3:27 ` [LTP] [RFC 1/3] syscalls/fanotify03: defined additional tcase members to support more tcase control Matthew Bobrowski
2018-10-24 3:27 ` [LTP] [RFC 2/3] syscalls/fanotify03: included execve() to generate_events() to increase test coverage Matthew Bobrowski
@ 2018-10-24 3:28 ` Matthew Bobrowski
2018-10-24 5:56 ` Amir Goldstein
2 siblings, 1 reply; 11+ messages in thread
From: Matthew Bobrowski @ 2018-10-24 3:28 UTC (permalink / raw)
To: ltp
* Defined FAN_OPEN_EXEC_PERM flag in local tests header file to
accommodate for the fact if it's not defined in system user space
headers
* Defined new tcase's for each mark type to support new fanotify flag
FAN_OPEN_EXEC_PERM tests
* Updated fanotify_mark failure logic to report the instance where
FAN_OPEN_EXEC_PERM flag is not available within the kernel
Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
---
testcases/kernel/syscalls/fanotify/fanotify.h | 14 ++++++
.../kernel/syscalls/fanotify/fanotify03.c | 45 ++++++++++++++++---
2 files changed, 53 insertions(+), 6 deletions(-)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 535f1cef2..b0a847ab6 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -60,6 +60,20 @@ static long fanotify_mark(int fd, unsigned int flags, uint64_t mask,
#ifndef FAN_MARK_FILESYSTEM
#define FAN_MARK_FILESYSTEM 0x00000100
#endif
+#ifndef FAN_OPEN_EXEC_PERM
+#define FAN_OPEN_EXEC_PERM 0x00040000
+#endif
+
+/*
+ * FAN_ALL_PERM_EVENTS has been deprecated, so any new permission events
+ * are not to be added to it. To cover the instance where a new permission
+ * event is defined, we need to update FAN_ALL_PERM_EVENTS accordingly,
+ * thus needing to do the undef/define. Any new permission events should
+ * be added to the newly defined macro below.
+ */
+#undef FAN_ALL_PERM_EVENTS
+#define FAN_ALL_PERM_EVENTS (FAN_OPEN_PERM | FAN_OPEN_EXEC_PERM | \
+ FAN_ACCESS_PERM)
struct fanotify_mark_type {
unsigned int flag;
diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
index f9418ee6b..d6d8fb8bf 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify03.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
@@ -59,7 +59,7 @@ static struct tcase {
struct event event_set[EVENT_MAX];
} tcases[] = {
{
- "inode mark permission events",
+ "inode mark, FAN_OPEN_PERM | FAN_ACCESS_PERM events",
INIT_FANOTIFY_MARK_TYPE(INODE),
FAN_OPEN_PERM | FAN_ACCESS_PERM, 3,
{
@@ -69,7 +69,16 @@ static struct tcase {
}
},
{
- "mount mark permission events",
+ "inode mark, FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM events",
+ INIT_FANOTIFY_MARK_TYPE(INODE),
+ FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM, 2,
+ {
+ {FAN_ACCESS_PERM, FAN_DENY},
+ {FAN_OPEN_EXEC_PERM, FAN_DENY}
+ }
+ },
+ {
+ "mount mark, FAN_OPEN_PERM | FAN_ACCESS_PERM events",
INIT_FANOTIFY_MARK_TYPE(MOUNT),
FAN_OPEN_PERM | FAN_ACCESS_PERM, 3,
{
@@ -79,7 +88,16 @@ static struct tcase {
}
},
{
- "filesystem mark permission events",
+ "mount mark, FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM events",
+ INIT_FANOTIFY_MARK_TYPE(MOUNT),
+ FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM, 2,
+ {
+ {FAN_ACCESS_PERM, FAN_DENY},
+ {FAN_OPEN_EXEC_PERM, FAN_DENY}
+ }
+ },
+ {
+ "filesystem mark, FAN_OPEN_PERM | FAN_ACCESS_PERM events",
INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
FAN_OPEN_PERM | FAN_ACCESS_PERM, 3,
{
@@ -87,7 +105,16 @@ static struct tcase {
{FAN_ACCESS_PERM, FAN_DENY},
{FAN_OPEN_PERM, FAN_DENY}
}
- }
+ },
+ {
+ "filesystem mark, FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM events",
+ INIT_FANOTIFY_MARK_TYPE(MOUNT),
+ FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM, 2,
+ {
+ {FAN_ACCESS_PERM, FAN_DENY},
+ {FAN_OPEN_EXEC_PERM, FAN_DENY}
+ }
+ },
};
static void generate_events(void)
@@ -182,7 +209,13 @@ static int setup_mark(unsigned int n)
if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
tc->mask, AT_FDCWD, files[i]) < 0) {
if (errno == EINVAL && support_perm_events &&
- mark->flag == FAN_MARK_FILESYSTEM) {
+ tc->mask & FAN_OPEN_EXEC_PERM) {
+ tst_res(TCONF,
+ "FAN_OPEN_EXEC_PERM not supported in "
+ "kernel?");
+ return -1;
+ } else if (errno == EINVAL && support_perm_events &&
+ mark->flag == FAN_MARK_FILESYSTEM) {
tst_res(TCONF,
"FAN_MARK_FILESYSTEM not supported in "
"kernel?");
@@ -193,7 +226,7 @@ static int setup_mark(unsigned int n)
"not configured in kernel?");
} else {
tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_ADD | %s, "
+ "fanotify_mark(%d, FAN_MARK_ADD | %s, "
"FAN_ACCESS_PERM | FAN_OPEN_PERM, "
"AT_FDCWD, %s) failed.",
fd_notify, mark->name, fname);
--
2.17.2
--
Matthew Bobrowski <mbobrowski@mbobrowski.org>
^ permalink raw reply related [flat|nested] 11+ messages in thread* [LTP] [RFC 3/3] syscalls/fanotify03: add FAN_OPEN_EXEC_PERM tcase support
2018-10-24 3:28 ` [LTP] [RFC 3/3] syscalls/fanotify03: add FAN_OPEN_EXEC_PERM tcase support Matthew Bobrowski
@ 2018-10-24 5:56 ` Amir Goldstein
2018-10-25 6:39 ` Matthew Bobrowski
0 siblings, 1 reply; 11+ messages in thread
From: Amir Goldstein @ 2018-10-24 5:56 UTC (permalink / raw)
To: ltp
On Wed, Oct 24, 2018 at 6:28 AM Matthew Bobrowski
<mbobrowski@mbobrowski.org> wrote:
>
> * Defined FAN_OPEN_EXEC_PERM flag in local tests header file to
> accommodate for the fact if it's not defined in system user space
> headers
>
> * Defined new tcase's for each mark type to support new fanotify flag
> FAN_OPEN_EXEC_PERM tests
>
> * Updated fanotify_mark failure logic to report the instance where
> FAN_OPEN_EXEC_PERM flag is not available within the kernel
>
> Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> ---
Very neat and small change - that's good!
Do you have the series on a public branch that I can test?
Nits below.
> testcases/kernel/syscalls/fanotify/fanotify.h | 14 ++++++
> .../kernel/syscalls/fanotify/fanotify03.c | 45 ++++++++++++++++---
> 2 files changed, 53 insertions(+), 6 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index 535f1cef2..b0a847ab6 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -60,6 +60,20 @@ static long fanotify_mark(int fd, unsigned int flags, uint64_t mask,
> #ifndef FAN_MARK_FILESYSTEM
> #define FAN_MARK_FILESYSTEM 0x00000100
> #endif
> +#ifndef FAN_OPEN_EXEC_PERM
> +#define FAN_OPEN_EXEC_PERM 0x00040000
> +#endif
> +
> +/*
> + * FAN_ALL_PERM_EVENTS has been deprecated, so any new permission events
> + * are not to be added to it. To cover the instance where a new permission
> + * event is defined, we need to update FAN_ALL_PERM_EVENTS accordingly,
> + * thus needing to do the undef/define. Any new permission events should
> + * be added to the newly defined macro below.
> + */
> +#undef FAN_ALL_PERM_EVENTS
Instead of undef/define, let's define and use LTP_ALL_PERM_EVENTS
that will be more clear to the reader IMO.
Comment above can stay mostly as it is.
> +#define FAN_ALL_PERM_EVENTS (FAN_OPEN_PERM | FAN_OPEN_EXEC_PERM | \
> + FAN_ACCESS_PERM)
>
> struct fanotify_mark_type {
> unsigned int flag;
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
> index f9418ee6b..d6d8fb8bf 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify03.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
+ /*
+ * Keep first FAN_OPEN_EXEC_PERM test case before first MARK_TYPE(FILESYSTEM)
+ * to allow for correct detection between exec events not supported
and filesystem marks
+ * not supported.
+ */
> @@ -59,7 +59,7 @@ static struct tcase {
> struct event event_set[EVENT_MAX];
> } tcases[] = {
> {
> - "inode mark permission events",
> + "inode mark, FAN_OPEN_PERM | FAN_ACCESS_PERM events",
> INIT_FANOTIFY_MARK_TYPE(INODE),
> FAN_OPEN_PERM | FAN_ACCESS_PERM, 3,
> {
> @@ -69,7 +69,16 @@ static struct tcase {
> }
> },
> {
> - "mount mark permission events",
> + "inode mark, FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM events",
> + INIT_FANOTIFY_MARK_TYPE(INODE),
> + FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM, 2,
> + {
> + {FAN_ACCESS_PERM, FAN_DENY},
> + {FAN_OPEN_EXEC_PERM, FAN_DENY}
> + }
> + },
> + {
> + "mount mark, FAN_OPEN_PERM | FAN_ACCESS_PERM events",
> INIT_FANOTIFY_MARK_TYPE(MOUNT),
> FAN_OPEN_PERM | FAN_ACCESS_PERM, 3,
> {
> @@ -79,7 +88,16 @@ static struct tcase {
> }
> },
> {
> - "filesystem mark permission events",
> + "mount mark, FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM events",
> + INIT_FANOTIFY_MARK_TYPE(MOUNT),
> + FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM, 2,
> + {
> + {FAN_ACCESS_PERM, FAN_DENY},
> + {FAN_OPEN_EXEC_PERM, FAN_DENY}
> + }
> + },
> + {
> + "filesystem mark, FAN_OPEN_PERM | FAN_ACCESS_PERM events",
> INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> FAN_OPEN_PERM | FAN_ACCESS_PERM, 3,
> {
> @@ -87,7 +105,16 @@ static struct tcase {
> {FAN_ACCESS_PERM, FAN_DENY},
> {FAN_OPEN_PERM, FAN_DENY}
> }
> - }
> + },
> + {
> + "filesystem mark, FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM events",
> + INIT_FANOTIFY_MARK_TYPE(MOUNT),
INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> + FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM, 2,
> + {
> + {FAN_ACCESS_PERM, FAN_DENY},
> + {FAN_OPEN_EXEC_PERM, FAN_DENY}
> + }
> + },
> };
>
> static void generate_events(void)
> @@ -182,7 +209,13 @@ static int setup_mark(unsigned int n)
> if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> tc->mask, AT_FDCWD, files[i]) < 0) {
> if (errno == EINVAL && support_perm_events &&
> - mark->flag == FAN_MARK_FILESYSTEM) {
> + tc->mask & FAN_OPEN_EXEC_PERM) {
Almost. It's hard to get this right ;-)
+ (tc->mask & FAN_OPEN_EXEC_PERM &&
!support_exec_events)) {
If EXEC events were proven to be supported by prior test case, we
won't emit this
warning and may very well fall through to report no FAN_MARK_FILESYSTEM support.
It's true that no upstream kernel is expected to support EXEC events
and not FILESYSTEM
marks, but who knows what future kernels this test will run on.
EXEC events are quite easy to backport to old kernels.
> + tst_res(TCONF,
> + "FAN_OPEN_EXEC_PERM not supported in "
> + "kernel?");
> + return -1;
> + } else if (errno == EINVAL && support_perm_events &&
> + mark->flag == FAN_MARK_FILESYSTEM) {
> tst_res(TCONF,
> "FAN_MARK_FILESYSTEM not supported in "
> "kernel?");
> @@ -193,7 +226,7 @@ static int setup_mark(unsigned int n)
> "not configured in kernel?");
> } else {
> tst_brk(TBROK | TERRNO,
> - "fanotify_mark (%d, FAN_MARK_ADD | %s, "
> + "fanotify_mark(%d, FAN_MARK_ADD | %s, "
> "FAN_ACCESS_PERM | FAN_OPEN_PERM, "
> "AT_FDCWD, %s) failed.",
> fd_notify, mark->name, fname);
}
+ }
- } else {
- /*
- * To distigouish between perm event not supported and
- * filesystem mark not supported.
- */
- support_perm_events = 1;
+ /*
+ * To distinguish between perm event not supported exec events not
+ * supported and filesystem mark not supported.
+ */
+ support_perm_events = 1;
+ if (tc->mask & FAN_OPEN_EXEC_PERM)
+ support_exec_events = 1;
- }
Thanks,
Amir.
^ permalink raw reply [flat|nested] 11+ messages in thread* [LTP] [RFC 3/3] syscalls/fanotify03: add FAN_OPEN_EXEC_PERM tcase support
2018-10-24 5:56 ` Amir Goldstein
@ 2018-10-25 6:39 ` Matthew Bobrowski
0 siblings, 0 replies; 11+ messages in thread
From: Matthew Bobrowski @ 2018-10-25 6:39 UTC (permalink / raw)
To: ltp
On Wed, Oct 24, 2018 at 08:56:45AM +0300, Amir Goldstein wrote:
> On Wed, Oct 24, 2018 at 6:28 AM Matthew Bobrowski
> <mbobrowski@mbobrowski.org> wrote:
> >
> > * Defined FAN_OPEN_EXEC_PERM flag in local tests header file to
> > accommodate for the fact if it's not defined in system user space
> > headers
> >
> > * Defined new tcase's for each mark type to support new fanotify flag
> > FAN_OPEN_EXEC_PERM tests
> >
> > * Updated fanotify_mark failure logic to report the instance where
> > FAN_OPEN_EXEC_PERM flag is not available within the kernel
> >
> > Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> > ---
>
> Very neat and small change - that's good!
> Do you have the series on a public branch that I can test?
>
I've pushed the branch with your recommended changes. You can find it
using the link below.
https://github.com/matthewbobrowski/ltp/tree/fanotify03_exec
>
> Nits below.
>
> > testcases/kernel/syscalls/fanotify/fanotify.h | 14 ++++++
> > .../kernel/syscalls/fanotify/fanotify03.c | 45 ++++++++++++++++---
> > 2 files changed, 53 insertions(+), 6 deletions(-)
> >
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> > index 535f1cef2..b0a847ab6 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> > @@ -60,6 +60,20 @@ static long fanotify_mark(int fd, unsigned int flags, uint64_t mask,
> > #ifndef FAN_MARK_FILESYSTEM
> > #define FAN_MARK_FILESYSTEM 0x00000100
> > #endif
> > +#ifndef FAN_OPEN_EXEC_PERM
> > +#define FAN_OPEN_EXEC_PERM 0x00040000
> > +#endif
> > +
> > +/*
> > + * FAN_ALL_PERM_EVENTS has been deprecated, so any new permission events
> > + * are not to be added to it. To cover the instance where a new permission
> > + * event is defined, we need to update FAN_ALL_PERM_EVENTS accordingly,
> > + * thus needing to do the undef/define. Any new permission events should
> > + * be added to the newly defined macro below.
> > + */
> > +#undef FAN_ALL_PERM_EVENTS
>
> Instead of undef/define, let's define and use LTP_ALL_PERM_EVENTS
> that will be more clear to the reader IMO.
> Comment above can stay mostly as it is.
>
I agree with this approach and I initially thought to do the same, but I
didn't want it to be wrong or deviate from using existing fanotify flag
names. Anyway, I've updated this as per recommendations.
> > +#define FAN_ALL_PERM_EVENTS (FAN_OPEN_PERM | FAN_OPEN_EXEC_PERM | \
> > + FAN_ACCESS_PERM)
> >
> > struct fanotify_mark_type {
> > unsigned int flag;
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
> > index f9418ee6b..d6d8fb8bf 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify03.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
>
> + /*
> + * Keep first FAN_OPEN_EXEC_PERM test case before first MARK_TYPE(FILESYSTEM)
> + * to allow for correct detection between exec events not supported
> and filesystem marks
> + * not supported.
> + */
>
Added.
> > @@ -59,7 +59,7 @@ static struct tcase {
> > struct event event_set[EVENT_MAX];
> > } tcases[] = {
> > {
> > - "inode mark permission events",
> > + "inode mark, FAN_OPEN_PERM | FAN_ACCESS_PERM events",
> > INIT_FANOTIFY_MARK_TYPE(INODE),
> > FAN_OPEN_PERM | FAN_ACCESS_PERM, 3,
> > {
> > @@ -69,7 +69,16 @@ static struct tcase {
> > }
> > },
> > {
> > - "mount mark permission events",
> > + "inode mark, FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM events",
> > + INIT_FANOTIFY_MARK_TYPE(INODE),
> > + FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM, 2,
> > + {
> > + {FAN_ACCESS_PERM, FAN_DENY},
> > + {FAN_OPEN_EXEC_PERM, FAN_DENY}
> > + }
> > + },
> > + {
> > + "mount mark, FAN_OPEN_PERM | FAN_ACCESS_PERM events",
> > INIT_FANOTIFY_MARK_TYPE(MOUNT),
> > FAN_OPEN_PERM | FAN_ACCESS_PERM, 3,
> > {
> > @@ -79,7 +88,16 @@ static struct tcase {
> > }
> > },
> > {
> > - "filesystem mark permission events",
> > + "mount mark, FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM events",
> > + INIT_FANOTIFY_MARK_TYPE(MOUNT),
> > + FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM, 2,
> > + {
> > + {FAN_ACCESS_PERM, FAN_DENY},
> > + {FAN_OPEN_EXEC_PERM, FAN_DENY}
> > + }
> > + },
> > + {
> > + "filesystem mark, FAN_OPEN_PERM | FAN_ACCESS_PERM events",
> > INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> > FAN_OPEN_PERM | FAN_ACCESS_PERM, 3,
> > {
> > @@ -87,7 +105,16 @@ static struct tcase {
> > {FAN_ACCESS_PERM, FAN_DENY},
> > {FAN_OPEN_PERM, FAN_DENY}
> > }
> > - }
> > + },
> > + {
> > + "filesystem mark, FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM events",
> > + INIT_FANOTIFY_MARK_TYPE(MOUNT),
> INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
>
> > + FAN_ACCESS_PERM | FAN_OPEN_EXEC_PERM, 2,
> > + {
> > + {FAN_ACCESS_PERM, FAN_DENY},
> > + {FAN_OPEN_EXEC_PERM, FAN_DENY}
> > + }
> > + },
> > };
> >
> > static void generate_events(void)
> > @@ -182,7 +209,13 @@ static int setup_mark(unsigned int n)
> > if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> > tc->mask, AT_FDCWD, files[i]) < 0) {
> > if (errno == EINVAL && support_perm_events &&
> > - mark->flag == FAN_MARK_FILESYSTEM) {
> > + tc->mask & FAN_OPEN_EXEC_PERM) {
>
> Almost. It's hard to get this right ;-)
>
> + (tc->mask & FAN_OPEN_EXEC_PERM &&
> !support_exec_events)) {
>
> If EXEC events were proven to be supported by prior test case, we
> won't emit this
> warning and may very well fall through to report no FAN_MARK_FILESYSTEM support.
> It's true that no upstream kernel is expected to support EXEC events
> and not FILESYSTEM
> marks, but who knows what future kernels this test will run on.
> EXEC events are quite easy to backport to old kernels.
>
Thanks :-)
I've updated it based on these recommendations.
> > + tst_res(TCONF,
> > + "FAN_OPEN_EXEC_PERM not supported in "
> > + "kernel?");
> > + return -1;
> > + } else if (errno == EINVAL && support_perm_events &&
> > + mark->flag == FAN_MARK_FILESYSTEM) {
> > tst_res(TCONF,
> > "FAN_MARK_FILESYSTEM not supported in "
> > "kernel?");
> > @@ -193,7 +226,7 @@ static int setup_mark(unsigned int n)
> > "not configured in kernel?");
> > } else {
> > tst_brk(TBROK | TERRNO,
> > - "fanotify_mark (%d, FAN_MARK_ADD | %s, "
> > + "fanotify_mark(%d, FAN_MARK_ADD | %s, "
> > "FAN_ACCESS_PERM | FAN_OPEN_PERM, "
> > "AT_FDCWD, %s) failed.",
> > fd_notify, mark->name, fname);
> }
> + }
> - } else {
> - /*
> - * To distigouish between perm event not supported and
> - * filesystem mark not supported.
> - */
> - support_perm_events = 1;
> + /*
> + * To distinguish between perm event not supported exec events not
> + * supported and filesystem mark not supported.
> + */
> + support_perm_events = 1;
> + if (tc->mask & FAN_OPEN_EXEC_PERM)
> + support_exec_events = 1;
> - }
>
Updated.
--
Matthew Bobrowski <mbobrowski@mbobrowski.org>
^ permalink raw reply [flat|nested] 11+ messages in thread