[LTP] [PATCH 0/4] fanotify: FAN_REPORT_FID and Directory Modification Events

[LTP] [PATCH 0/4] fanotify: FAN_REPORT_FID and Directory Modification Events

[Matthew Bobrowski]

This patch series contains the changes needed to support and validate the new
FAN_REPORT_FID flag and directory modification event functionality within the
fanotify API.

Amir Goldstein (1):
  syscalls/fanotify01: add FAN_REPORT_FID test cases

Matthew Bobrowski (3):
  syscalls/fanotify13: new test to verify FAN_REPORT_FID functionality
  syscalls/fanotify14: new test to validate FAN_REPORT_FID interface
    return values
  syscalls/fanotify15: verify fid for dirent events

 testcases/kernel/syscalls/fanotify/.gitignore   |   3 +
 testcases/kernel/syscalls/fanotify/fanotify.h   |  41 +++
 testcases/kernel/syscalls/fanotify/fanotify01.c |  41 ++-
 testcases/kernel/syscalls/fanotify/fanotify13.c | 329 ++++++++++++++++++++++++
 testcases/kernel/syscalls/fanotify/fanotify14.c | 170 ++++++++++++
 testcases/kernel/syscalls/fanotify/fanotify15.c | 256 ++++++++++++++++++
 6 files changed, 838 insertions(+), 2 deletions(-)
 create mode 100644 testcases/kernel/syscalls/fanotify/fanotify13.c
 create mode 100644 testcases/kernel/syscalls/fanotify/fanotify14.c
 create mode 100644 testcases/kernel/syscalls/fanotify/fanotify15.c

-- 
2.16.4


-- 
Matthew Bobrowski

[LTP] [PATCH 1/4] syscalls/fanotify01: add FAN_REPORT_FID test cases

[Amir Goldstein]

For now, only check that event->fd is FAN_NOFD.
On old kernels the new test with result in TCONF.

tmpfs doesn't support FAN_REPORT_FID (it has zero fsid), so use a
blockdev filesystem. It is better to use a blockdev filesystem for
the rest of the test cases anyway to get more diversity in testing.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 testcases/kernel/syscalls/fanotify/fanotify.h   |  3 ++
 testcases/kernel/syscalls/fanotify/fanotify01.c | 41 +++++++++++++++++++++++--
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index e5cc59232..14654b7c7 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -57,6 +57,9 @@ static long fanotify_mark(int fd, unsigned int flags, uint64_t mask,
 #ifndef FAN_REPORT_TID
 #define FAN_REPORT_TID		0x00000100
 #endif
+#ifndef FAN_REPORT_FID
+#define FAN_REPORT_FID		0x00000200
+#endif
 
 #ifndef FAN_MARK_INODE
 #define FAN_MARK_INODE		0
diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
index 9b590746e..44966dce5 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify01.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
@@ -37,18 +37,37 @@
 static struct tcase {
 	const char *tname;
 	struct fanotify_mark_type mark;
+	unsigned int init_flags;
 } tcases[] = {
 	{
 		"inode mark events",
 		INIT_FANOTIFY_MARK_TYPE(INODE),
+		FAN_CLASS_NOTIF
 	},
 	{
 		"mount mark events",
 		INIT_FANOTIFY_MARK_TYPE(MOUNT),
+		FAN_CLASS_NOTIF
 	},
 	{
 		"filesystem mark events",
 		INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
+		FAN_CLASS_NOTIF
+	},
+	{
+		"inode mark events (FAN_REPORT_FID)",
+		INIT_FANOTIFY_MARK_TYPE(INODE),
+		FAN_CLASS_NOTIF|FAN_REPORT_FID
+	},
+	{
+		"mount mark events (FAN_REPORT_FID)",
+		INIT_FANOTIFY_MARK_TYPE(MOUNT),
+		FAN_CLASS_NOTIF|FAN_REPORT_FID
+	},
+	{
+		"filesystem mark events (FAN_REPORT_FID)",
+		INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
+		FAN_CLASS_NOTIF|FAN_REPORT_FID
 	},
 };
 
@@ -69,7 +88,18 @@ static void test_fanotify(unsigned int n)
 
 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
 
-	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
+	fd_notify = fanotify_init(tc->init_flags, O_RDONLY);
+	if (fd_notify < 0) {
+		if (errno == EINVAL &&
+		    (tc->init_flags & FAN_REPORT_FID)) {
+			tst_res(TCONF,
+				"FAN_REPORT_FID not supported in kernel?");
+			return;
+		}
+		tst_brk(TBROK | TERRNO,
+			"fanotify_init (0x%x, O_RDONLY) "
+			"failed", tc->init_flags);
+	}
 
 	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
 			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
@@ -265,7 +295,8 @@ static void test_fanotify(unsigned int n)
 				(unsigned)getpid(),
 				event->fd);
 		} else {
-			if (event->fd == -2)
+			if (event->fd == -2 || (event->fd == FAN_NOFD &&
+			    (tc->init_flags & FAN_REPORT_FID)))
 				goto pass;
 			ret = read(event->fd, buf, BUF_SIZE);
 			if (ret != (int)strlen(fname)) {
@@ -327,6 +358,12 @@ pass:
 
 static void setup(void)
 {
+	int fd;
+
+	/* Check for kernel fanotify support */
+	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
+	SAFE_CLOSE(fd);
+
 	sprintf(fname, MOUNT_PATH"/tfile_%d", getpid());
 	SAFE_FILE_PRINTF(fname, "1");
 }
-- 
2.16.4


-- 
Matthew Bobrowski

[LTP] [PATCH 1/4] syscalls/fanotify01: add FAN_REPORT_FID test cases

[Cyril Hrubis]

Hi!
Applied, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/4] syscalls/fanotify13: new test to verify FAN_REPORT_FID functionality

[Matthew Bobrowski]

Newly defined test file to validate the fanotify FAN_REPORT_FID
functionality.

Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 testcases/kernel/syscalls/fanotify/.gitignore   |   1 +
 testcases/kernel/syscalls/fanotify/fanotify.h   |  19 +-
 testcases/kernel/syscalls/fanotify/fanotify13.c | 329 ++++++++++++++++++++++++
 3 files changed, 346 insertions(+), 3 deletions(-)
 create mode 100644 testcases/kernel/syscalls/fanotify/fanotify13.c

diff --git a/testcases/kernel/syscalls/fanotify/.gitignore b/testcases/kernel/syscalls/fanotify/.gitignore
index 4256b8cd3..16bdd99e5 100644
--- a/testcases/kernel/syscalls/fanotify/.gitignore
+++ b/testcases/kernel/syscalls/fanotify/.gitignore
@@ -10,4 +10,5 @@
 /fanotify10
 /fanotify11
 /fanotify12
+/fanotify13
 /fanotify_child
diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 14654b7c7..450f3829d 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -57,9 +57,6 @@ static long fanotify_mark(int fd, unsigned int flags, uint64_t mask,
 #ifndef FAN_REPORT_TID
 #define FAN_REPORT_TID		0x00000100
 #endif
-#ifndef FAN_REPORT_FID
-#define FAN_REPORT_FID		0x00000200
-#endif
 
 #ifndef FAN_MARK_INODE
 #define FAN_MARK_INODE		0
@@ -89,6 +86,22 @@ struct fanotify_mark_type {
 	const char * name;
 };
 
+#ifndef FAN_REPORT_FID
+#define FAN_REPORT_FID		0x00000200
+
+struct fanotify_event_info_header {
+	uint8_t info_type;
+	uint8_t pad;
+	uint16_t len;
+};
+
+struct fanotify_event_info_fid {
+	struct fanotify_event_info_header hdr;
+	__kernel_fsid_t fsid;
+	unsigned char handle[0];
+};
+#endif
+
 #define INIT_FANOTIFY_MARK_TYPE(t) \
 	{ FAN_MARK_ ## t, "FAN_MARK_" #t }
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
new file mode 100644
index 000000000..fb49df5f9
--- /dev/null
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -0,0 +1,329 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 Matthew Bobrowski. All Rights Reserved.
+ *
+ * Started by Matthew Bobrowski <mbobrowski@mbobrowski.org>
+ *
+ * DESCRIPTION
+ *	Validate that the values returned within an event when
+ *	FAN_REPORT_FID is specified matches those that are obtained via
+ *	explicit invocation to system calls statfs(2) and
+ *	name_to_handle_at(2).
+ */
+#define _GNU_SOURCE
+#include "config.h"
+
+#include <stdio.h>
+#include <string.h>
+#include <sys/statfs.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <unistd.h>
+#include "tst_test.h"
+#include "fanotify.h"
+
+#if defined(HAVE_SYS_FANOTIFY_H)
+#include <sys/fanotify.h>
+
+#define PATH_LEN 128
+#define BUF_SIZE 256
+#define DIR_ONE "dir_one"
+#define FILE_ONE "file_one"
+#define FILE_TWO "file_two"
+#define MOUNT_PATH "mntpoint"
+#define EVENT_MAX ARRAY_SIZE(objects)
+#define DIR_PATH_ONE MOUNT_PATH"/"DIR_ONE
+#define FILE_PATH_ONE MOUNT_PATH"/"FILE_ONE
+#define FILE_PATH_TWO MOUNT_PATH"/"FILE_TWO
+
+struct event_t {
+	unsigned long long expected_mask;
+	__kernel_fsid_t fsid;
+	struct file_handle handle;
+	char buf[MAX_HANDLE_SZ];
+};
+
+static struct object_t {
+	const char *path;
+	int is_dir;
+} objects[] = {
+	{FILE_PATH_ONE, 0},
+	{FILE_PATH_TWO, 0},
+	{DIR_PATH_ONE, 1}
+};
+
+static struct test_case_t {
+	struct fanotify_mark_type mark;
+	unsigned long long mask;
+} test_cases[] = {
+	{
+		INIT_FANOTIFY_MARK_TYPE(INODE),
+		FAN_OPEN | FAN_CLOSE_NOWRITE
+	},
+	{
+		INIT_FANOTIFY_MARK_TYPE(INODE),
+		FAN_OPEN | FAN_CLOSE_NOWRITE | FAN_ONDIR
+	},
+	{
+		INIT_FANOTIFY_MARK_TYPE(MOUNT),
+		FAN_OPEN | FAN_CLOSE_NOWRITE
+	},
+	{
+		INIT_FANOTIFY_MARK_TYPE(MOUNT),
+		FAN_OPEN | FAN_CLOSE_NOWRITE | FAN_ONDIR
+	},
+	{
+		INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
+		FAN_OPEN | FAN_CLOSE_NOWRITE
+	},
+	{
+		INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
+		FAN_OPEN | FAN_CLOSE_NOWRITE | FAN_ONDIR
+	}
+};
+
+static int fanotify_fd;
+static char events_buf[BUF_SIZE];
+static struct event_t event_set[EVENT_MAX];
+
+static void create_objects(void)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(objects); i++) {
+		if (objects[i].is_dir)
+			SAFE_MKDIR(objects[i].path, 0755);
+		else
+			SAFE_FILE_PRINTF(objects[i].path, "0");
+	}
+}
+
+static void do_setup(void)
+{
+	int fd;
+
+	/* Check for kernel fanotify support */
+	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
+	SAFE_CLOSE(fd);
+
+	/* Create file and directories for testing */
+	create_objects();
+}
+
+static void get_object_stats(struct test_case_t *tc)
+{
+	int mount_id;
+	unsigned int i;
+	struct statfs stats;
+
+	for (i = 0; i < ARRAY_SIZE(objects); i++) {
+		if (statfs(objects[i].path, &stats) == -1)
+			tst_brk(TBROK | TERRNO,
+				"statfs(%s, ...) failed", objects[i].path);
+		memcpy(&event_set[i].fsid, &stats.f_fsid,
+			sizeof(stats.f_fsid));
+
+		event_set[i].handle.handle_bytes = MAX_HANDLE_SZ;
+		if (name_to_handle_at(AT_FDCWD, objects[i].path,
+					&event_set[i].handle,
+					&mount_id, 0) == -1) {
+			if (errno == EOPNOTSUPP) {
+				tst_res(TCONF,
+					"filesystem %s does not support file "
+					"handles",
+					tst_device->fs_type);
+			}
+			tst_brk(TBROK | TERRNO,
+				"name_to_handle_at(AT_FDCWD, %s, ...) failed",
+				objects[i].path);
+		}
+
+		event_set[i].expected_mask = tc->mask;
+		if (!objects[i].is_dir)
+			event_set[i].expected_mask &= ~FAN_ONDIR;
+	}
+}
+
+static int setup_marks(unsigned int fd, struct test_case_t *tc)
+{
+	unsigned int i;
+	struct fanotify_mark_type *mark = &tc->mark;
+
+	for (i = 0; i < ARRAY_SIZE(objects); i++) {
+		if (fanotify_mark(fd, FAN_MARK_ADD | mark->flag, tc->mask,
+					AT_FDCWD, objects[i].path) == -1) {
+			if (errno == EINVAL &&
+				mark->flag & FAN_MARK_FILESYSTEM) {
+				tst_res(TCONF,
+					"FAN_MARK_FILESYSTEM not supported by "
+					"kernel");
+				return 1;
+			} else if (errno == ENODEV &&
+					!event_set[i].fsid.val[0] &&
+					!event_set[i].fsid.val[1]) {
+				tst_res(TCONF,
+					"FAN_REPORT_FID not supported on "
+					"filesystem type %s",
+					tst_device->fs_type);
+				return 1;
+			}
+			tst_brk(TBROK | TERRNO,
+				"fanotify_mark(%d, FAN_MARK_ADD, FAN_OPEN, "
+				"AT_FDCWD, %s) failed",
+				fanotify_fd, objects[i].path);
+		}
+	}
+	return 0;
+}
+
+static void do_test(unsigned int number)
+{
+	unsigned int i;
+	int len, fds[ARRAY_SIZE(objects)];
+
+	struct file_handle *event_file_handle;
+	struct fanotify_event_metadata *metadata;
+	struct fanotify_event_info_fid *event_fid;
+	struct test_case_t *tc = &test_cases[number];
+	struct fanotify_mark_type *mark = &tc->mark;
+
+	tst_res(TINFO,
+		"Test #%d: FAN_REPORT_FID with mark flag: %s",
+		number, mark->name);
+
+	/* Gets the filesystem fsid and file handle for each object */
+	get_object_stats(tc);
+
+	fanotify_fd = fanotify_init(FAN_CLASS_NOTIF | FAN_REPORT_FID, O_RDONLY);
+	if (fanotify_fd == -1) {
+		if (errno == EINVAL) {
+			tst_res(TCONF,
+				"FAN_REPORT_FID not supported by kernel");
+			return;
+		}
+		tst_brk(TBROK | TERRNO,
+			"fanotify_init(FAN_CLASS_NOTIF | FAN_REPORT_FID, "
+			"O_RDONLY) failed");
+	}
+
+	/* Place marks on a set of objects */
+	if (setup_marks(fanotify_fd, tc) != 0)
+		return;
+
+	/* Generate sequence of FAN_OPEN events on objects */
+	for (i = 0; i < ARRAY_SIZE(objects); i++)
+		fds[i] = SAFE_OPEN(objects[i].path, O_RDONLY);
+
+	/* Generate sequence of FAN_CLOSE_NOWRITE events on objects. Each
+	 * FAN_CLOSE_NOWRITE event is expected to be merged with its
+	 * respective FAN_OPEN event that was performed on the same object
+	 */
+	for (i = 0; i < ARRAY_SIZE(objects); i++) {
+		if (fds[i] > 0)
+			SAFE_CLOSE(fds[i]);
+	}
+
+	/* Read events from event queue */
+	len = SAFE_READ(0, fanotify_fd, events_buf, BUF_SIZE);
+
+	/* Iterate over event queue */
+	for (i = 0, metadata = (struct fanotify_event_metadata *) events_buf;
+		FAN_EVENT_OK(metadata, len);
+		metadata = FAN_EVENT_NEXT(metadata, len), i++) {
+		event_fid = (struct fanotify_event_info_fid *) (metadata + 1);
+		event_file_handle = (struct file_handle *) event_fid->handle;
+
+		/* File descriptor is redundant with FAN_REPORT_FID */
+		if (metadata->fd != FAN_NOFD)
+			tst_res(TFAIL,
+				"Unexpectedly received file descriptor %d in "
+				"event. Expected to get FAN_NOFD(%d)",
+				metadata->fd, FAN_NOFD);
+
+		/* Ensure that the correct mask has been reported in event */
+		if (metadata->mask != event_set[i].expected_mask)
+			tst_res(TFAIL,
+				"Unexpected mask received: %llx (expected: "
+				"%llx) in event",
+				metadata->mask,
+				event_set[i].expected_mask);
+
+		/* Verify handle_bytes returned in event */
+		if (event_file_handle->handle_bytes
+				!= event_set[i].handle.handle_bytes) {
+			tst_res(TFAIL,
+				"handle_bytes (%x) returned in event does not "
+				"equal to handle_bytes (%x) returned in "
+				"name_to_handle_at(2)",
+				event_file_handle->handle_bytes,
+				event_set[i].handle.handle_bytes);
+			continue;
+		}
+
+		/* Verify handle_type returned in event */
+		if (event_file_handle->handle_type !=
+				event_set[i].handle.handle_type) {
+			tst_res(TFAIL,
+				"handle_type (%x) returned in event does not "
+				"equal to handle_type (%x) returned in "
+				"name_to_handle_at(2)",
+				event_file_handle->handle_type,
+				event_set[i].handle.handle_type);
+			continue;
+		}
+
+		/* Verify file identifier f_handle returned in event */
+		if (memcmp(event_file_handle->f_handle,
+				event_set[i].handle.f_handle,
+				event_set[i].handle.handle_bytes) != 0) {
+			tst_res(TFAIL,
+				"event_file_handle->f_handle does not match "
+				"event_set[i].handle.f_handle returned in "
+				"name_to_handle_at(2)");
+			continue;
+		}
+
+		/* Verify filesystem ID fsid  returned in event */
+		if (memcmp(&event_fid->fsid, &event_set[i].fsid,
+				sizeof(event_set[i].fsid)) != 0) {
+			tst_res(TFAIL,
+				"event_fid.fsid != stat.f_fsid that was "
+				"obtained via statfs(2)");
+			continue;
+		}
+
+		tst_res(TPASS,
+			"got event: mask=%llx, pid=%d, fid=%x.%x.%lx values "
+			"returned in event match those returned in statfs(2) "
+			"and name_to_handle_at(2)",
+			metadata->mask,
+			getpid(),
+			event_fid->fsid.val[0],
+			event_fid->fsid.val[1],
+			*(unsigned long *) event_file_handle->f_handle);
+	}
+}
+
+static void do_cleanup(void)
+{
+	if (fanotify_fd > 0)
+		SAFE_CLOSE(fanotify_fd);
+}
+
+static struct tst_test test = {
+	.setup = do_setup,
+	.test = do_test,
+	.tcnt = ARRAY_SIZE(test_cases),
+	.cleanup = do_cleanup,
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = MOUNT_PATH,
+	.all_filesystems = 1
+};
+
+#else
+	TST_TEST_CONF("System does not have required fanotify support");
+#endif
-- 
2.16.4


-- 
Matthew Bobrowski

[LTP] [PATCH 2/4] syscalls/fanotify13: new test to verify FAN_REPORT_FID functionality

[Cyril Hrubis]

Hi!
> Newly defined test file to validate the fanotify FAN_REPORT_FID
> functionality.
> 
> Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  testcases/kernel/syscalls/fanotify/.gitignore   |   1 +
>  testcases/kernel/syscalls/fanotify/fanotify.h   |  19 +-
>  testcases/kernel/syscalls/fanotify/fanotify13.c | 329 ++++++++++++++++++++++++

This is missing the runtest entry in runtest/syscalls so that the test
is picked up by the testrunner.

And the same bug slipped in during review for fanotify12 so I've
commited a fix that adds it there.

...

> +static void do_test(unsigned int number)
> +{
> +	unsigned int i;
> +	int len, fds[ARRAY_SIZE(objects)];
> +
> +	struct file_handle *event_file_handle;
> +	struct fanotify_event_metadata *metadata;
> +	struct fanotify_event_info_fid *event_fid;
> +	struct test_case_t *tc = &test_cases[number];
> +	struct fanotify_mark_type *mark = &tc->mark;
> +
> +	tst_res(TINFO,
> +		"Test #%d: FAN_REPORT_FID with mark flag: %s",
> +		number, mark->name);
> +
> +	/* Gets the filesystem fsid and file handle for each object */
> +	get_object_stats(tc);

Is there a reason why are are not calling this function once in the test
setup? It will be called for every test iteration when the test is
passed the -i option...


Otherwise it looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/4] syscalls/fanotify13: new test to verify FAN_REPORT_FID functionality

[Amir Goldstein]

On Tue, Apr 16, 2019 at 4:41 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > Newly defined test file to validate the fanotify FAN_REPORT_FID
> > functionality.
> >
> > Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  testcases/kernel/syscalls/fanotify/.gitignore   |   1 +
> >  testcases/kernel/syscalls/fanotify/fanotify.h   |  19 +-
> >  testcases/kernel/syscalls/fanotify/fanotify13.c | 329 ++++++++++++++++++++++++
>
> This is missing the runtest entry in runtest/syscalls so that the test
> is picked up by the testrunner.
>
> And the same bug slipped in during review for fanotify12 so I've
> commited a fix that adds it there.
>
> ...
>
> > +static void do_test(unsigned int number)
> > +{
> > +     unsigned int i;
> > +     int len, fds[ARRAY_SIZE(objects)];
> > +
> > +     struct file_handle *event_file_handle;
> > +     struct fanotify_event_metadata *metadata;
> > +     struct fanotify_event_info_fid *event_fid;
> > +     struct test_case_t *tc = &test_cases[number];
> > +     struct fanotify_mark_type *mark = &tc->mark;
> > +
> > +     tst_res(TINFO,
> > +             "Test #%d: FAN_REPORT_FID with mark flag: %s",
> > +             number, mark->name);
> > +
> > +     /* Gets the filesystem fsid and file handle for each object */
> > +     get_object_stats(tc);
>
> Is there a reason why are are not calling this function once in the test
> setup? It will be called for every test iteration when the test is
> passed the -i option...
>

I think that's an oversight. It could be called once during setup.

Thanks,
Amir.

[LTP] [PATCH 2/4] syscalls/fanotify13: new test to verify FAN_REPORT_FID functionality

[Matthew Bobrowski]

On Tue, Apr 16, 2019 at 05:23:14PM +0300, Amir Goldstein wrote:
> On Tue, Apr 16, 2019 at 4:41 PM Cyril Hrubis <chrubis@suse.cz> wrote:
> >
> > Hi!
> > > Newly defined test file to validate the fanotify FAN_REPORT_FID
> > > functionality.
> > >
> > > Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> > > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> > >  testcases/kernel/syscalls/fanotify/.gitignore   |   1 +
> > >  testcases/kernel/syscalls/fanotify/fanotify.h   |  19 +-
> > >  testcases/kernel/syscalls/fanotify/fanotify13.c | 329 ++++++++++++++++++++++++
> >
> > This is missing the runtest entry in runtest/syscalls so that the test
> > is picked up by the testrunner.
> >
> > And the same bug slipped in during review for fanotify12 so I've
> > commited a fix that adds it there.
> >
> > ...
> >

My apologies, this was a simple oversight. I've gone ahead and updated it
accordingly.

> > > +static void do_test(unsigned int number)
> > > +{
> > > +     unsigned int i;
> > > +     int len, fds[ARRAY_SIZE(objects)];
> > > +
> > > +     struct file_handle *event_file_handle;
> > > +     struct fanotify_event_metadata *metadata;
> > > +     struct fanotify_event_info_fid *event_fid;
> > > +     struct test_case_t *tc = &test_cases[number];
> > > +     struct fanotify_mark_type *mark = &tc->mark;
> > > +
> > > +     tst_res(TINFO,
> > > +             "Test #%d: FAN_REPORT_FID with mark flag: %s",
> > > +             number, mark->name);
> > > +
> > > +     /* Gets the filesystem fsid and file handle for each object */
> > > +     get_object_stats(tc);
> >
> > Is there a reason why are are not calling this function once in the test
> > setup? It will be called for every test iteration when the test is
> > passed the -i option...
> >
> 
> I think that's an oversight. It could be called once during setup.

Indeed. I've changed this so that the function is now only called once from
within do_setup(). There was one code snippet (added below) that I had to move
out from get_object_stats() as there's a setup dependency for each test case.

--
event_set[i].expected_mask = tc->mask;
if (!objects[i].is_dir)
        event_set[i].expected_mask &= ~FAN_ONDIR;
--

I've added this snippet within setup_marks() as we're already iterating over
the objects array and passing in a task_struct_t object, which means we could
also setup the expected mask for each expected event there. Unless anyone
objects?

-- 
Matthew Bobrowski

[LTP] [PATCH 2/4] syscalls/fanotify13: new test to verify FAN_REPORT_FID functionality

[Amir Goldstein]

On Fri, Apr 19, 2019 at 11:37 AM Matthew Bobrowski
<mbobrowski@mbobrowski.org> wrote:
>
> On Tue, Apr 16, 2019 at 05:23:14PM +0300, Amir Goldstein wrote:
> > On Tue, Apr 16, 2019 at 4:41 PM Cyril Hrubis <chrubis@suse.cz> wrote:
> > >
> > > Hi!
> > > > Newly defined test file to validate the fanotify FAN_REPORT_FID
> > > > functionality.
> > > >
> > > > Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> > > > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > > > ---
> > > >  testcases/kernel/syscalls/fanotify/.gitignore   |   1 +
> > > >  testcases/kernel/syscalls/fanotify/fanotify.h   |  19 +-
> > > >  testcases/kernel/syscalls/fanotify/fanotify13.c | 329 ++++++++++++++++++++++++
> > >
> > > This is missing the runtest entry in runtest/syscalls so that the test
> > > is picked up by the testrunner.
> > >
> > > And the same bug slipped in during review for fanotify12 so I've
> > > commited a fix that adds it there.
> > >
> > > ...
> > >
>
> My apologies, this was a simple oversight. I've gone ahead and updated it
> accordingly.
>
> > > > +static void do_test(unsigned int number)
> > > > +{
> > > > +     unsigned int i;
> > > > +     int len, fds[ARRAY_SIZE(objects)];
> > > > +
> > > > +     struct file_handle *event_file_handle;
> > > > +     struct fanotify_event_metadata *metadata;
> > > > +     struct fanotify_event_info_fid *event_fid;
> > > > +     struct test_case_t *tc = &test_cases[number];
> > > > +     struct fanotify_mark_type *mark = &tc->mark;
> > > > +
> > > > +     tst_res(TINFO,
> > > > +             "Test #%d: FAN_REPORT_FID with mark flag: %s",
> > > > +             number, mark->name);
> > > > +
> > > > +     /* Gets the filesystem fsid and file handle for each object */
> > > > +     get_object_stats(tc);
> > >
> > > Is there a reason why are are not calling this function once in the test
> > > setup? It will be called for every test iteration when the test is
> > > passed the -i option...
> > >
> >
> > I think that's an oversight. It could be called once during setup.
>
> Indeed. I've changed this so that the function is now only called once from
> within do_setup(). There was one code snippet (added below) that I had to move
> out from get_object_stats() as there's a setup dependency for each test case.
>
> --
> event_set[i].expected_mask = tc->mask;
> if (!objects[i].is_dir)
>         event_set[i].expected_mask &= ~FAN_ONDIR;
> --
>
> I've added this snippet within setup_marks() as we're already iterating over
> the objects array and passing in a task_struct_t object, which means we could
> also setup the expected mask for each expected event there. Unless anyone
> objects?
>

Sounds good to me.

Thanks,
Amir.

[LTP] [PATCH 3/4] syscalls/fanotify14: new test to validate FAN_REPORT_FID interface return values

[Matthew Bobrowski]

New test file has been introduced to validate that the fanotify interface
returns the correct error values upon specifying invalid flags and masks
in conjunction with FAN_REPORT_FID.

Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 testcases/kernel/syscalls/fanotify/.gitignore   |   1 +
 testcases/kernel/syscalls/fanotify/fanotify.h   |  25 ++++
 testcases/kernel/syscalls/fanotify/fanotify14.c | 170 ++++++++++++++++++++++++
 3 files changed, 196 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fanotify/fanotify14.c

diff --git a/testcases/kernel/syscalls/fanotify/.gitignore b/testcases/kernel/syscalls/fanotify/.gitignore
index 16bdd99e5..bf389c96a 100644
--- a/testcases/kernel/syscalls/fanotify/.gitignore
+++ b/testcases/kernel/syscalls/fanotify/.gitignore
@@ -11,4 +11,5 @@
 /fanotify11
 /fanotify12
 /fanotify13
+/fanotify14
 /fanotify_child
diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 450f3829d..46a4b6289 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -64,6 +64,31 @@ static long fanotify_mark(int fd, unsigned int flags, uint64_t mask,
 #ifndef FAN_MARK_FILESYSTEM
 #define FAN_MARK_FILESYSTEM	0x00000100
 #endif
+/* New dirent event masks */
+#ifndef FAN_ATTRIB
+#define FAN_ATTRIB		0x00000004
+#endif
+#ifndef FAN_MOVED_FROM
+#define FAN_MOVED_FROM		0x00000040
+#endif
+#ifndef FAN_MOVED_TO
+#define FAN_MOVED_TO		0x00000080
+#endif
+#ifndef FAN_CREATE
+#define FAN_CREATE		0x00000100
+#endif
+#ifndef FAN_DELETE
+#define FAN_DELETE		0x00000200
+#endif
+#ifndef FAN_DELETE_SELF
+#define FAN_DELETE_SELF		0x00000400
+#endif
+#ifndef FAN_MOVE_SELF
+#define FAN_MOVE_SELF		0x00000800
+#endif
+#ifndef FAN_MOVE
+#define FAN_MOVE		(FAN_MOVED_FROM | FAN_MOVED_TO)
+#endif
 #ifndef FAN_OPEN_EXEC
 #define FAN_OPEN_EXEC		0x00001000
 #endif
diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
new file mode 100644
index 000000000..b80eac99f
--- /dev/null
+++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 Matthew Bobrowski. All Rights Reserved.
+ *
+ * Started by Matthew Bobrowski <mbobrowski@mbobrowski.org>
+ *
+ * DESCRIPTION
+ *	This test file has been designed to ensure that the fanotify
+ *	system calls fanotify_init(2) and fanotify_mark(2) return the
+ *	correct error code to the calling process when an invalid flag or
+ *	mask value has been specified in conjunction with FAN_REPORT_FID.
+ */
+
+#include "tst_test.h"
+#include "fanotify.h"
+
+#include <errno.h>
+
+#if defined(HAVE_SYS_FANOTIFY_H)
+#include <sys/fanotify.h>
+
+#define MNTPOINT "mntpoint"
+#define FILE1 MNTPOINT"/file1"
+
+/* List of inode events that are only available when notification group is
+ * set to report fid
+ */
+#define INODE_EVENTS (FAN_ATTRIB | FAN_CREATE | FAN_DELETE | FAN_MOVE | \
+		      FAN_DELETE_SELF | FAN_MOVE_SELF)
+
+static int fanotify_fd;
+
+/* Each test case has been designed in a manner whereby the values defined
+ * within should result in the interface to return an error to the calling
+ * process.
+ */
+static struct test_case_t {
+	unsigned int init_flags;
+	unsigned int mark_flags;
+	unsigned long long mask;
+} test_cases[] = {
+	{
+		FAN_CLASS_CONTENT | FAN_REPORT_FID, 0, 0
+	},
+	{
+		FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID, 0, 0
+	},
+	{
+		FAN_CLASS_NOTIF, 0, INODE_EVENTS
+	},
+	{
+		FAN_CLASS_NOTIF | FAN_REPORT_FID, FAN_MARK_MOUNT, INODE_EVENTS
+	}
+};
+
+static void do_setup(void)
+{
+	int fd;
+
+	/* Check for kernel fanotify support */
+	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
+	SAFE_CLOSE(fd);
+
+	/* Create temporary test file to place marks on */
+	SAFE_FILE_PRINTF(FILE1, "0");
+}
+
+static void do_test(unsigned int number)
+{
+	int ret;
+	struct test_case_t *tc = &test_cases[number];
+
+	fanotify_fd = fanotify_init(tc->init_flags, O_RDONLY);
+
+	if (fanotify_fd < 0) {
+		/* EINVAL is to be returned to the calling process when
+		 * an invalid notification class is specified in
+		 * conjunction with FAN_REPORT_FID
+		 */
+		if (errno == EINVAL) {
+			tst_res(TPASS,
+				"fanotify_fd=%d, fanotify_init(%x, O_RDONLY) "
+				"failed with error EINVAL as expected",
+				fanotify_fd,
+				tc->init_flags);
+			return;
+		}
+		tst_brk(TBROK | TERRNO,
+			"fanotify_fd=%d, fanotify_init(%x, O_RDONLY) failed",
+			fanotify_fd,
+			tc->init_flags);
+	}
+
+	/* A test case with a mask set to zero indicate that they've been
+	 * specifically designed to test and fail on the fanotify_init()
+	 * system call.
+	 */
+	if (tc->mask == 0) {
+		tst_res(TFAIL,
+			"fanotify_fd=%d fanotify_init(%x, O_RDONLY) "
+			"unexpectedly succeeded when tests with mask 0 are"
+			"expected to fail when calling fanotify_init()",
+			fanotify_fd,
+			tc->init_flags);
+		return;
+	}
+
+	ret = fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark_flags,
+				tc->mask, AT_FDCWD, FILE1);
+
+	if (ret < 0) {
+		/* EINVAL is to be returned to the calling process when
+		 * attempting to use INODE_EVENTS without FAN_REPORT_FID
+		 * specified on the notification group, or using
+		 * INODE_EVENTS with mark type FAN_MARK_MOUNT.
+		 */
+		if (errno == EINVAL) {
+			tst_res(TPASS,
+				"ret=%d, fanotify_mark(%d, FAN_MARK_ADD | %x, "
+				"%llx, AT_FDCWD, %s) failed with error EINVAL "
+				"as expected",
+				ret,
+				fanotify_fd,
+				tc->mark_flags,
+				tc->mask,
+				FILE1);
+			return;
+		}
+		tst_brk(TBROK | TERRNO,
+			"ret=%d, fanotify_mark(%d, FAN_MARK_ADD | %x, %llx, "
+			"AT_FDCWD, %s) failed",
+			ret,
+			fanotify_fd,
+			tc->mark_flags,
+			tc->mask,
+			FILE1);
+	}
+
+	tst_res(TFAIL,
+		"fanotify_fd=%d, ret=%d, fanotify_init(%x, O_RDONLY) and "
+		"fanotify_mark(%d, FAN_MARK_ADD | %x, %llx, AT_FDCWD, %s) did "
+		"not return any errors as expected",
+		fanotify_fd,
+		ret,
+		tc->init_flags,
+		fanotify_fd,
+		tc->mark_flags,
+		tc->mask,
+		FILE1);
+}
+
+static void do_cleanup(void)
+{
+	if (fanotify_fd > 0)
+		SAFE_CLOSE(fanotify_fd);
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.setup = do_setup,
+	.test = do_test,
+	.tcnt = ARRAY_SIZE(test_cases),
+	.cleanup = do_cleanup,
+	.mount_device = 1,
+	.mntpoint = MNTPOINT
+};
+
+#else
+	TST_TEST_CONF("System does not have required fanotify support")
+#endif
-- 
2.16.4


-- 
Matthew Bobrowski

[LTP] [PATCH 3/4] syscalls/fanotify14: new test to validate FAN_REPORT_FID interface return values

[Cyril Hrubis]

Hi!
This one is missing runtest entry as well, otherwise looks fine.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 3/4] syscalls/fanotify14: new test to validate FAN_REPORT_FID interface return values

[Matthew Bobrowski]

On Tue, Apr 16, 2019 at 04:04:34PM +0200, Cyril Hrubis wrote:
> Hi!
> This one is missing runtest entry as well, otherwise looks fine.

Simple oversight. I've updated the commit accordingly. I will post through a
new patch series with all the fixes shortly.

-- 
Matthew Bobrowski

[LTP] [PATCH 4/4] syscalls/fanotify15: verify fid for dirent events

[Matthew Bobrowski]

New test file that provides coverage for new dirent events.

Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 testcases/kernel/syscalls/fanotify/.gitignore   |   1 +
 testcases/kernel/syscalls/fanotify/fanotify15.c | 256 ++++++++++++++++++++++++
 2 files changed, 257 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fanotify/fanotify15.c

diff --git a/testcases/kernel/syscalls/fanotify/.gitignore b/testcases/kernel/syscalls/fanotify/.gitignore
index bf389c96a..68e4cc7aa 100644
--- a/testcases/kernel/syscalls/fanotify/.gitignore
+++ b/testcases/kernel/syscalls/fanotify/.gitignore
@@ -12,4 +12,5 @@
 /fanotify12
 /fanotify13
 /fanotify14
+/fanotify15
 /fanotify_child
diff --git a/testcases/kernel/syscalls/fanotify/fanotify15.c b/testcases/kernel/syscalls/fanotify/fanotify15.c
new file mode 100644
index 000000000..83062443b
--- /dev/null
+++ b/testcases/kernel/syscalls/fanotify/fanotify15.c
@@ -0,0 +1,256 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 CTERA Networks. All Rights Reserved.
+ *
+ * Started by Amir Goldstein <amir73il@gmail.com>
+ * Modified by Matthew Bobrowski <mbobrowski@mbobrowski.org>
+ *
+ * DESCRIPTION
+ *	Test file that has been purposely designed to verify
+ *	FAN_REPORT_FID functionality while using newly defined dirent
+ *	events.
+ */
+#define _GNU_SOURCE
+#include "config.h"
+
+#include <string.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <sys/statfs.h>
+#include <sys/types.h>
+
+#include "tst_test.h"
+#include "fanotify.h"
+
+#if defined(HAVE_SYS_FANOTIFY_H)
+#include <sys/fanotify.h>
+
+#define BUF_SIZE 256
+#define EVENT_MAX 256 
+
+#define MOUNT_POINT "mntpoint"
+#define TEST_DIR MOUNT_POINT"/test_dir"
+#define DIR1 TEST_DIR"/dir1"
+#define DIR2 TEST_DIR"/dir2"
+#define FILE1 TEST_DIR"/file1"
+#define FILE2 TEST_DIR"/file2"
+
+struct event_t {
+	unsigned long long mask;
+	__kernel_fsid_t fsid;
+	struct file_handle handle;
+	char buf[MAX_HANDLE_SZ];
+};
+
+static int fanotify_fd;
+static char events_buf[BUF_SIZE];
+static struct event_t event_set[EVENT_MAX];
+
+static void do_setup(void)
+{
+	int fd;
+
+	/* Check kernel for fanotify support */
+	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
+	SAFE_CLOSE(fd);
+
+	fanotify_fd = fanotify_init(FAN_REPORT_FID, O_RDONLY);
+	if (fanotify_fd == -1) {
+		if (errno == EINVAL)
+			tst_brk(TCONF,
+				"FAN_REPORT_FID not supported in kernel");
+		tst_brk(TBROK | TERRNO,
+			"fanotify_init(FAN_REPORT_FID, O_RDONLY) failed");
+	}
+
+	SAFE_MKDIR(TEST_DIR, 0755);
+}
+
+static void get_fid_data(int i, const char *path)
+{
+	int mount_id;
+	struct statfs stats;
+
+	if (statfs(path, &stats) == -1)
+		tst_brk(TBROK | TERRNO,
+			"statfs(%s, ...) failed", path);
+	memcpy(&event_set[i].fsid, &stats.f_fsid, sizeof(stats.f_fsid));
+
+	event_set[i].handle.handle_bytes = MAX_HANDLE_SZ;
+	if (name_to_handle_at(AT_FDCWD, path, &event_set[i].handle,
+				&mount_id, 0) == -1) {
+		if (errno == EOPNOTSUPP) {
+			tst_brk(TCONF,
+				"filesystem %s does not support file handles",
+				tst_device->fs_type);
+		}
+		tst_brk(TBROK | TERRNO,
+			"name_to_handle_at(AT_FDCWD, %s, ...) failed",
+			path);
+	}
+}
+
+static void do_test(void)
+{
+	int i, fd, len, count = 0;
+
+	struct file_handle *event_file_handle;
+	struct fanotify_event_metadata *metadata;
+	struct fanotify_event_info_fid *event_fid;
+
+	if (fanotify_mark(fanotify_fd, FAN_MARK_ADD | FAN_MARK_FILESYSTEM,
+				FAN_CREATE | FAN_DELETE | FAN_ATTRIB |
+				FAN_MOVED_FROM | FAN_MOVED_TO | 
+				FAN_DELETE_SELF | FAN_ONDIR,
+				AT_FDCWD, TEST_DIR) == -1) {
+		tst_brk(TBROK | TERRNO,
+			"fanotify_mark(%d, FAN_MARK_ADD, FAN_CREATE | "
+			"FAN_DELETE | FAN_MOVED_FROM | FAN_MOVED_TO | "
+			"FAN_DELETE_SELF | FAN_ONDIR, AT_FDCWD, %s) failed",
+			fanotify_fd, TEST_DIR);
+	}
+
+	/* Generate a sequence of events */
+	event_set[count].mask = FAN_CREATE | FAN_MOVED_FROM | FAN_MOVED_TO | \
+				FAN_DELETE;
+	get_fid_data(count, TEST_DIR);
+	count++;
+
+	fd = SAFE_CREAT(FILE1, 0644);
+	SAFE_CLOSE(fd);
+
+	SAFE_RENAME(FILE1, FILE2);
+
+	event_set[count].mask = FAN_ATTRIB | FAN_DELETE_SELF;
+	get_fid_data(count, FILE2);
+	count++;
+
+	SAFE_UNLINK(FILE2);
+
+	/* Generate a sequence of events on a directory. Subsequent events
+	 * are merged, so it's required that we set FAN_ONDIR once in
+	 * order to acknowledge that changes related to a subdirectory
+	 * took place. Events on subdirectories are not merged with events
+	 * on non-subdirectories.
+	 */	
+	event_set[count].mask = FAN_ONDIR | FAN_CREATE | FAN_MOVED_FROM | \
+				FAN_MOVED_TO | FAN_DELETE;
+	get_fid_data(count, TEST_DIR);
+	count++;
+
+	SAFE_MKDIR(DIR1, 0755);
+
+	SAFE_RENAME(DIR1, DIR2);
+
+	event_set[count].mask = FAN_ONDIR | FAN_DELETE_SELF;
+	get_fid_data(count, DIR2);
+	count++;
+
+	SAFE_RMDIR(DIR2);
+
+	/* Read events from the event queue */
+	len = SAFE_READ(0, fanotify_fd, events_buf, BUF_SIZE);
+
+	/* Process each event in buffer */
+	for (i = 0, metadata = (struct fanotify_event_metadata *) events_buf;
+		FAN_EVENT_OK(metadata, len);
+		metadata = FAN_EVENT_NEXT(metadata,len), i++) {
+		event_fid = (struct fanotify_event_info_fid *) (metadata + 1);
+		event_file_handle = (struct file_handle *) event_fid->handle;
+
+		if (i >= count) {
+			tst_res(TFAIL,
+				"got unnecessary event: mask=%llx "
+				"pid=%u fd=%d",
+				(unsigned long long) metadata->mask,
+				metadata->pid,
+				metadata->fd);
+			metadata->mask = 0;
+		} else if (metadata->fd != FAN_NOFD) {
+			tst_res(TFAIL,
+				"Received unexpected file descriptor %d in "
+				"event. Expected to get FAN_NOFD(%d)",
+				metadata->fd, FAN_NOFD);
+		} else if (metadata->mask != event_set[i].mask) {
+			tst_res(TFAIL,
+				"Got event: mask=%llx (expected %llx) "
+				"pid=%u fd=%d",
+				(unsigned long long) metadata->mask,
+				event_set[i].mask,
+				(unsigned) metadata->pid,
+				metadata->fd);
+		} else if (metadata->pid != getpid()) {
+			tst_res(TFAIL,
+				"Got event: mask=%llx pid=%u "
+				"(expected %u) fd=%d",
+				(unsigned long long) metadata->mask,
+				(unsigned) metadata->pid,
+				(unsigned) getpid(),
+				metadata->fd);
+		} else if (event_file_handle->handle_bytes !=
+				event_set[i].handle.handle_bytes) {
+			tst_res(TFAIL,
+				"Got event: handle_bytes (%x) returned in "
+				"event does not equal handle_bytes (%x) "
+				"retunred in name_to_handle_at(2)",
+				event_file_handle->handle_bytes,
+				event_set[i].handle.handle_bytes);
+		} else if (event_file_handle->handle_type !=
+				event_set[i].handle.handle_type) {
+			tst_res(TFAIL,
+				"handle_type (%x) returned in event does not "
+				"equal to handle_type (%x) returned in "
+				"name_to_handle_at(2)",
+				event_file_handle->handle_type,
+				event_set[i].handle.handle_type);
+		} else if (memcmp(event_file_handle->f_handle,
+					event_set[i].handle.f_handle,
+					event_set[i].handle.handle_bytes) 
+					!= 0) {
+			tst_res(TFAIL,
+				"event_file_handle->f_handle does not match "
+				"handle.f_handle returned in "
+				"name_to_handle_at(2)"); 
+		} else if (memcmp(&event_fid->fsid, &event_set[i].fsid, 
+					sizeof(event_set[i].fsid)) != 0) {
+			tst_res(TFAIL,
+				"event_fid->fsid != stats.f_fsid that was "
+				"obtained via statfs(2)"); 
+		} else {
+			tst_res(TPASS,
+				"Got event: mask=%llx, pid=%u, "
+				"fid=%x.%x.%lx values",
+				metadata->mask,
+				getpid(),
+				event_fid->fsid.val[0],
+				event_fid->fsid.val[1],
+				*(unsigned long *)
+				event_file_handle->f_handle);
+		}
+	}
+
+	for (; i < count; i++)
+		tst_res(TFAIL,
+			"Didn't receive event: mask=%llx",
+			event_set[i].mask);
+}
+
+static void do_cleanup(void)
+{
+	if (fanotify_fd > 0)
+		SAFE_CLOSE(fanotify_fd);
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = MOUNT_POINT,
+	.setup = do_setup,
+	.test_all = do_test,
+	.cleanup = do_cleanup
+};
+
+#else
+	TST_TEST_CONF("System does not have required fanotify support");
+#endif
-- 
2.16.4


-- 
Matthew Bobrowski

[LTP] [PATCH 4/4] syscalls/fanotify15: verify fid for dirent events

[Cyril Hrubis]

Hi!
This one misses runtest entry as well.

> +#define BUF_SIZE 256
> +#define EVENT_MAX 256 
> +
> +#define MOUNT_POINT "mntpoint"
> +#define TEST_DIR MOUNT_POINT"/test_dir"
> +#define DIR1 TEST_DIR"/dir1"
> +#define DIR2 TEST_DIR"/dir2"
> +#define FILE1 TEST_DIR"/file1"
> +#define FILE2 TEST_DIR"/file2"
> +
> +struct event_t {
> +	unsigned long long mask;
> +	__kernel_fsid_t fsid;
> +	struct file_handle handle;
> +	char buf[MAX_HANDLE_SZ];
> +};
> +
> +static int fanotify_fd;
> +static char events_buf[BUF_SIZE];
> +static struct event_t event_set[EVENT_MAX];
> +
> +static void do_setup(void)
> +{
> +	int fd;
> +
> +	/* Check kernel for fanotify support */
> +	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
> +	SAFE_CLOSE(fd);
> +
> +	fanotify_fd = fanotify_init(FAN_REPORT_FID, O_RDONLY);
> +	if (fanotify_fd == -1) {
> +		if (errno == EINVAL)
> +			tst_brk(TCONF,
> +				"FAN_REPORT_FID not supported in kernel");
> +		tst_brk(TBROK | TERRNO,
> +			"fanotify_init(FAN_REPORT_FID, O_RDONLY) failed");
> +	}
> +
> +	SAFE_MKDIR(TEST_DIR, 0755);
> +}
> +
> +static void get_fid_data(int i, const char *path)
> +{
> +	int mount_id;
> +	struct statfs stats;
> +
> +	if (statfs(path, &stats) == -1)
> +		tst_brk(TBROK | TERRNO,
> +			"statfs(%s, ...) failed", path);
> +	memcpy(&event_set[i].fsid, &stats.f_fsid, sizeof(stats.f_fsid));
> +
> +	event_set[i].handle.handle_bytes = MAX_HANDLE_SZ;
> +	if (name_to_handle_at(AT_FDCWD, path, &event_set[i].handle,
> +				&mount_id, 0) == -1) {
> +		if (errno == EOPNOTSUPP) {
> +			tst_brk(TCONF,
> +				"filesystem %s does not support file handles",
> +				tst_device->fs_type);
> +		}
> +		tst_brk(TBROK | TERRNO,
> +			"name_to_handle_at(AT_FDCWD, %s, ...) failed",
> +			path);
> +	}
> +}

Can put this code into an header in the fanotify/ directory along with
the event_t structure definiton so that we do not carry two copies of
nearly identical code.

If we passed a pointer to the event_t structure instead of the index we
can call this function in a loop in the get_object_stats() in
fanotify13, right?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 4/4] syscalls/fanotify15: verify fid for dirent events

[Amir Goldstein]

On Tue, Apr 16, 2019 at 5:30 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> This one misses runtest entry as well.
>
> > +#define BUF_SIZE 256
> > +#define EVENT_MAX 256
> > +
> > +#define MOUNT_POINT "mntpoint"
> > +#define TEST_DIR MOUNT_POINT"/test_dir"
> > +#define DIR1 TEST_DIR"/dir1"
> > +#define DIR2 TEST_DIR"/dir2"
> > +#define FILE1 TEST_DIR"/file1"
> > +#define FILE2 TEST_DIR"/file2"
> > +
> > +struct event_t {
> > +     unsigned long long mask;
> > +     __kernel_fsid_t fsid;
> > +     struct file_handle handle;
> > +     char buf[MAX_HANDLE_SZ];
> > +};
> > +
> > +static int fanotify_fd;
> > +static char events_buf[BUF_SIZE];
> > +static struct event_t event_set[EVENT_MAX];
> > +
> > +static void do_setup(void)
> > +{
> > +     int fd;
> > +
> > +     /* Check kernel for fanotify support */
> > +     fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
> > +     SAFE_CLOSE(fd);
> > +
> > +     fanotify_fd = fanotify_init(FAN_REPORT_FID, O_RDONLY);
> > +     if (fanotify_fd == -1) {
> > +             if (errno == EINVAL)
> > +                     tst_brk(TCONF,
> > +                             "FAN_REPORT_FID not supported in kernel");
> > +             tst_brk(TBROK | TERRNO,
> > +                     "fanotify_init(FAN_REPORT_FID, O_RDONLY) failed");
> > +     }
> > +
> > +     SAFE_MKDIR(TEST_DIR, 0755);
> > +}
> > +
> > +static void get_fid_data(int i, const char *path)
> > +{
> > +     int mount_id;
> > +     struct statfs stats;
> > +
> > +     if (statfs(path, &stats) == -1)
> > +             tst_brk(TBROK | TERRNO,
> > +                     "statfs(%s, ...) failed", path);
> > +     memcpy(&event_set[i].fsid, &stats.f_fsid, sizeof(stats.f_fsid));
> > +
> > +     event_set[i].handle.handle_bytes = MAX_HANDLE_SZ;
> > +     if (name_to_handle_at(AT_FDCWD, path, &event_set[i].handle,
> > +                             &mount_id, 0) == -1) {
> > +             if (errno == EOPNOTSUPP) {
> > +                     tst_brk(TCONF,
> > +                             "filesystem %s does not support file handles",
> > +                             tst_device->fs_type);
> > +             }
> > +             tst_brk(TBROK | TERRNO,
> > +                     "name_to_handle_at(AT_FDCWD, %s, ...) failed",
> > +                     path);
> > +     }
> > +}
>
> Can put this code into an header in the fanotify/ directory along with
> the event_t structure definiton so that we do not carry two copies of
> nearly identical code.

Correct, but I would rather leave the event_t structure private to the test
because it may have other fields in future tests of similar pattern...

>
> If we passed a pointer to the event_t structure instead of the index we
> can call this function in a loop in the get_object_stats() in
> fanotify13, right?
>

...so I'd prefer to pass a pointer to fsid and file_handle to helper instead,
something like:
void fanotify_get_fid(const char *path, __kernel_fsid_t *fsid, and
struct file_handle *handle)

Thanks,
Amir.

[LTP] [PATCH 4/4] syscalls/fanotify15: verify fid for dirent events

[Cyril Hrubis]

Hi!
> > Can put this code into an header in the fanotify/ directory along with
> > the event_t structure definiton so that we do not carry two copies of
> > nearly identical code.
> 
> Correct, but I would rather leave the event_t structure private to the test
> because it may have other fields in future tests of similar pattern...
> 
> >
> > If we passed a pointer to the event_t structure instead of the index we
> > can call this function in a loop in the get_object_stats() in
> > fanotify13, right?
> >
> 
> ...so I'd prefer to pass a pointer to fsid and file_handle to helper instead,
> something like:
> void fanotify_get_fid(const char *path, __kernel_fsid_t *fsid, and
> struct file_handle *handle)

Sounds good to me as well.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 4/4] syscalls/fanotify15: verify fid for dirent events

[Amir Goldstein]

On Tue, Apr 16, 2019 at 6:00 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > > Can put this code into an header in the fanotify/ directory along with
> > > the event_t structure definiton so that we do not carry two copies of
> > > nearly identical code.
> >
> > Correct, but I would rather leave the event_t structure private to the test
> > because it may have other fields in future tests of similar pattern...
> >
> > >
> > > If we passed a pointer to the event_t structure instead of the index we
> > > can call this function in a loop in the get_object_stats() in
> > > fanotify13, right?
> > >
> >
> > ...so I'd prefer to pass a pointer to fsid and file_handle to helper instead,
> > something like:
> > void fanotify_get_fid(const char *path, __kernel_fsid_t *fsid, and
> > struct file_handle *handle)
>
> Sounds good to me as well.
>

BTW, I think fanotify14/fanotify15 could use
.all_filesystems = 1

The file handle/fsid issues are filesystem specific (e.g. ENODEV error).

Is anybody else seeing these errors with all_filesystems?

tst_test.c:1157: INFO: Testing on xfs
tst_mkfs.c:90: INFO: Formatting /dev/loop0 with xfs opts='' extra opts=''
mkfs.xfs: Use the -f option to force overwrite.
tst_mkfs.c:101: BROK: mkfs.xfs:1: tst_test.c failed with 751

Thanks,
Amir.

[LTP] [PATCH 4/4] syscalls/fanotify15: verify fid for dirent events

[Cyril Hrubis]

Hi!
> Is anybody else seeing these errors with all_filesystems?
> 
> tst_test.c:1157: INFO: Testing on xfs
> tst_mkfs.c:90: INFO: Formatting /dev/loop0 with xfs opts='' extra opts=''
> mkfs.xfs: Use the -f option to force overwrite.
> tst_mkfs.c:101: BROK: mkfs.xfs:1: tst_test.c failed with 751

That looks strange. If I'm looking at this correctly the 751 is returned
from tst_run_cmd() which basically returns the value from waitpid() but
we call WEXITSTATUS() there before we return the number, which should
return only least significant 8 bits.

So as far as I can tell this output is impossible.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 4/4] syscalls/fanotify15: verify fid for dirent events

[Amir Goldstein]

On Thu, Apr 18, 2019 at 9:58 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > Is anybody else seeing these errors with all_filesystems?
> >
> > tst_test.c:1157: INFO: Testing on xfs
> > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with xfs opts='' extra opts=''
> > mkfs.xfs: Use the -f option to force overwrite.
> > tst_mkfs.c:101: BROK: mkfs.xfs:1: tst_test.c failed with 751
>
> That looks strange. If I'm looking at this correctly the 751 is returned
> from tst_run_cmd() which basically returns the value from waitpid() but
> we call WEXITSTATUS() there before we return the number, which should
> return only least significant 8 bits.
>
> So as far as I can tell this output is impossible.
>

Don't know about that, but the fact is that mkfs.xfs refuses to format
a device that has an existing filesystem on it without the -f flag.

Does that ring a bell?
I think it may be because I upgraded xfsprogs to v4.20.
Maybe that's a mkfs.xfs bug.

Thanks,
Amir.

[LTP] [PATCH 4/4] syscalls/fanotify15: verify fid for dirent events

[Cyril Hrubis]

Hi!
> Don't know about that, but the fact is that mkfs.xfs refuses to format
> a device that has an existing filesystem on it without the -f flag.
> 
> Does that ring a bell?

It does, but we clean up first 512kB of the block device prior to call
to mkfs.xfs exactly to avoid that.

> I think it may be because I upgraded xfsprogs to v4.20.

Quite likely.

> Maybe that's a mkfs.xfs bug.

Possibly.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 4/4] syscalls/fanotify15: verify fid for dirent events

[Amir Goldstein]

On Thu, Apr 18, 2019 at 10:42 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > Don't know about that, but the fact is that mkfs.xfs refuses to format
> > a device that has an existing filesystem on it without the -f flag.
> >
> > Does that ring a bell?
>
> It does, but we clean up first 512kB of the block device prior to call
> to mkfs.xfs exactly to avoid that.
>
> > I think it may be because I upgraded xfsprogs to v4.20.
>
> Quite likely.
>
> > Maybe that's a mkfs.xfs bug.
>
> Possibly.
>

No. seems like a local problem.
I must have built mkfs.xfs without ENABLE_BLKID.

All good now.

Thanks,
Amir.