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

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.

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.

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

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?