fanotify: FAN_REPORT_FID and Directory Modification Events

Newly defined test file to validate the fanotify FAN_REPORT_FID functionality.

Signed-off-by Matthew Bobrowski <[email protected]>

Reviewed-by Amir Goldstein <[email protected]>

---
 testcases/kernel/syscalls/fanotify/.gitignore   |   1 +
 testcases/kernel/syscalls/fanotify/fanotify.h   |  19 +-
 testcases/kernel/syscalls/fanotify/fanotify13.c | 329 ++++++++++++++++++++++++

testcases/kernel/syscalls/fanotify/fanotify13.c new file +7 -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;

Hi!

Quoted 9 lines

Newly defined test file to validate the fanotify FAN_REPORT_FID functionality.

Signed-off-by Matthew Bobrowski <[email protected]>

Reviewed-by Amir Goldstein <[email protected]>

---
 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.

...

Quoted 13 lines

testcases/kernel/syscalls/fanotify/fanotify13.c new file +13 -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",

testcases/kernel/syscalls/fanotify/fanotify13.c line 197

Quoted 4 lines

testcases/kernel/syscalls/fanotify/fanotify13.c new file +4 -0

+             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...

Quoted 12 lines · Cyril Hrubis

Hi!

Quoted 9 lines

Newly defined test file to validate the fanotify FAN_REPORT_FID functionality.

Signed-off-by Matthew Bobrowski <[email protected]>

Reviewed-by Amir Goldstein <[email protected]>

---
 testcases/kernel/syscalls/fanotify/.gitignore   |   1 +
 testcases/kernel/syscalls/fanotify/fanotify.h   |  19 +-
 testcases/kernel/syscalls/fanotify/fanotify13.c | 329 ++++++++++++++++++++++++

Quoted 8 lines

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.

...

Quoted 13 lines

Quoted 13 lines

testcases/kernel/syscalls/fanotify/fanotify13.c new file +13 -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",

Quoted 9 lines

testcases/kernel/syscalls/fanotify/fanotify13.c line 197

Quoted 4 lines

testcases/kernel/syscalls/fanotify/fanotify13.c new file +4 -0

+             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.

Quoted 13 lines · Amir Goldstein

Quoted 12 lines · Cyril Hrubis

Hi!

Quoted 9 lines

Newly defined test file to validate the fanotify FAN_REPORT_FID functionality.

Signed-off-by Matthew Bobrowski <[email protected]>

Reviewed-by Amir Goldstein <[email protected]>

---
 testcases/kernel/syscalls/fanotify/.gitignore   |   1 +
 testcases/kernel/syscalls/fanotify/fanotify.h   |  19 +-
 testcases/kernel/syscalls/fanotify/fanotify13.c | 329 ++++++++++++++++++++++++

Quoted 8 lines

Quoted 8 lines

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.

Quoted 23 lines

Quoted 13 lines

Quoted 13 lines

testcases/kernel/syscalls/fanotify/fanotify13.c new file +13 -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",

Quoted 9 lines

testcases/kernel/syscalls/fanotify/fanotify13.c line 197

Quoted 4 lines

testcases/kernel/syscalls/fanotify/fanotify13.c new file +4 -0

+             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...

Quoted 1 line

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?

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

testcases/kernel/syscalls/fanotify/fanotify15.c new file +54 -0

+#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) {

Hi!
This one misses runtest entry as well.

Quoted 60 lines

testcases/kernel/syscalls/fanotify/fanotify15.c new file +60 -0

+#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,

testcases/kernel/syscalls/fanotify/fanotify15.c line 91

Quoted 4 lines

testcases/kernel/syscalls/fanotify/fanotify15.c new file +4 -0

+                     "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?

Quoted 3 lines

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...

Quoted 5 lines

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)

Hi!

Quoted 13 lines

Quoted 3 lines

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...

Quoted 5 lines

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?

Quoted 4 lines

...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.

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

Hi!

Quoted 6 lines

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.

Hi!

Quoted 4 lines

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.

Quoted 1 line

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

Quite likely.

Quoted 1 line

Maybe that's a mkfs.xfs bug.

Possibly.