[LTP] [RFC 3/3] syscalls/fanotify03: add FAN_OPEN_EXEC_PERM tcase support
Matthew Bobrowski
mbobrowski@mbobrowski.org
Thu Oct 25 08:39:00 CEST 2018
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>
More information about the ltp
mailing list