[LTP] [RFC 3/3] syscalls/fanotify03: add FAN_OPEN_EXEC_PERM tcase support

Amir Goldstein amir73il@gmail.com
Wed Oct 24 07:56:45 CEST 2018


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.


More information about the ltp mailing list