FAN_REPORT_FID and Directory Modification Events

runtest/syscalls +1 -0

index 2b8ca71..dfdc6cb 100644
@@ -537,6 +537,7 @@ fanotify09 fanotify09
 fanotify10 fanotify10
 fanotify11 fanotify11
 fanotify12 fanotify12
+fanotify13 fanotify13
 
 ioperm01 ioperm01
 ioperm02 ioperm02

testcases/kernel/syscalls/fanotify/.gitignore +1 -0

index 4256b8c..16bdd99 100644
@@ -10,4 +10,5 @@
 /fanotify10
 /fanotify11
 /fanotify12
+/fanotify13
 /fanotify_child

testcases/kernel/syscalls/fanotify/fanotify.h +31 -3

index 14654b7..e9b23cc 100644
@@ -29,6 +29,11 @@
 #define	__FANOTIFY_H__
 
 #include "config.h"
+#include <sys/statfs.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <fcntl.h>
 
 #if defined(HAVE_SYS_FANOTIFY_H)
 

@@ -57,9 +62,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 +91,50 @@ 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];
+};
+
+/*
+ * Helper function used to obtain __kernel_fsid_t and file_handle objects
+ * for a given path. Used by test files correlated to FAN_REPORT_FID
+ * functionality.
+ */
+static inline void fanotify_get_fid(const char *path, __kernel_fsid_t *fsid,
+			struct file_handle *handle)
+{
+	int mount_id;
+	struct statfs stats;
+

testcases/kernel/syscalls/fanotify/fanotify.h +8 -0

+		}
+		tst_brk(TBROK | TERRNO,
+			"name_to_handle_at(AT_FDCWD, %s, ...) failed", path);
+	}
+}
+
+#endif
+
 #define INIT_FANOTIFY_MARK_TYPE(t) \
 	{ FAN_MARK_ ## t, "FAN_MARK_" #t }

Hi!
I've tried these tests on buildservice to check that there are no problems on slightly older distributions and found two.

The first one is that we do define fallback definitions in fanotify.h but then ifdef the whole test code in #if defined(HAVE_SYS_FANOTIFY_H) so that it's efectively disabled on older distros even with fallback definitions in place. Also it's TST_TEST_TCONF() but that is just easy to fix typo. I guess that we can remove the ifdef and sys/fanotify.h include from the test sources since we conditionally include the sys/fanotify.h in the local fanotify.h already.

The second one is that we fail to compile on older distributions because of missing name_to_handle_at() so we need configure check for that syscall and fallback definition in lapi/ header, or at least configure check and ifdef in the fanotify_get_fid() function. Which should be as easy as adding a name_to_handle_at line to AC_CHECK_FUNCS() in the configure.ac and using the macro from config.h.

Quoted 84 lines

runtest/syscalls +1 -0

index 2b8ca71..dfdc6cb 100644
@@ -537,6 +537,7 @@ fanotify09 fanotify09
 fanotify10 fanotify10
 fanotify11 fanotify11
 fanotify12 fanotify12
+fanotify13 fanotify13
 
 ioperm01 ioperm01
 ioperm02 ioperm02

testcases/kernel/syscalls/fanotify/.gitignore +1 -0

index 4256b8c..16bdd99 100644
@@ -10,4 +10,5 @@
 /fanotify10
 /fanotify11
 /fanotify12
+/fanotify13
 /fanotify_child

testcases/kernel/syscalls/fanotify/fanotify.h +37 -3

index 14654b7..e9b23cc 100644
@@ -29,6 +29,11 @@
 #define	__FANOTIFY_H__
 
 #include "config.h"
+#include <sys/statfs.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <fcntl.h>
 
 #if defined(HAVE_SYS_FANOTIFY_H)
 

@@ -57,9 +62,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 +91,50 @@ 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];
+};
+
+/*
+ * Helper function used to obtain __kernel_fsid_t and file_handle objects
+ * for a given path. Used by test files correlated to FAN_REPORT_FID
+ * functionality.
+ */
+static inline void fanotify_get_fid(const char *path, __kernel_fsid_t *fsid,
+			struct file_handle *handle)
+{
+	int mount_id;
+	struct statfs stats;
+
+	if (statfs(path, &stats) == -1)
+		tst_brk(TBROK | TERRNO,
+			"statfs(%s, ...) failed", path);
+	memcpy(fsid, &stats.f_fsid, sizeof(stats.f_fsid));
+
+	if (name_to_handle_at(AT_FDCWD, path, handle, &mount_id, 0) == -1) {

testcases/kernel/syscalls/fanotify/fanotify.h struct fanotify_mark_type { · line 129

Quoted 4 lines

testcases/kernel/syscalls/fanotify/fanotify.h +4 -0

+		if (errno == EOPNOTSUPP) {
+			tst_res(TCONF,
+				"filesystem %s does not support file handles",
+				tst_device->fs_type);

Btw, here the tst_res() does not make much sense sice the code will continue and we will exit the test with the tst_brk() below. Shouldn't we use tst_brk() here as well?

Quoted 10 lines

testcases/kernel/syscalls/fanotify/fanotify.h +8 -0

+		}
+		tst_brk(TBROK | TERRNO,
+			"name_to_handle_at(AT_FDCWD, %s, ...) failed", path);
+	}
+}
+
+#endif
+
 #define INIT_FANOTIFY_MARK_TYPE(t) \
 	{ FAN_MARK_ ## t, "FAN_MARK_" #t }

Quoted 11 lines · Cyril Hrubis

Hi!
I've tried these tests on buildservice to check that there are no problems on slightly older distributions and found two.

The first one is that we do define fallback definitions in fanotify.h but then ifdef the whole test code in #if defined(HAVE_SYS_FANOTIFY_H) so that it's efectively disabled on older distros even with fallback definitions in place. Also it's TST_TEST_TCONF() but that is just easy to fix typo. I guess that we can remove the ifdef and sys/fanotify.h include from the test sources since we conditionally include the sys/fanotify.h in the local fanotify.h already.

OK. In that case I can write a patch that is to be applied prior to this series which essentially just removes this specific preprocessor conditional directive from all the source files. Would you like me to do this?

Quoted 6 lines

The second one is that we fail to compile on older distributions because of missing name_to_handle_at() so we need configure check for that syscall and fallback definition in lapi/ header, or at least configure check and ifdef in the fanotify_get_fid() function. Which should be as easy as adding a name_to_handle_at line to AC_CHECK_FUNCS() in the configure.ac and using the macro from config.h.

Sure. I've gone ahead an updated it to accommodate for this. Prior to submitting through another patches series, changes can be found here:
https://github.com/matthewbobrowski/ltp/commit/54264db0e574d2f90e716a510fcb1da11ee174dc.

I think we can do better and also provide a fallback definition though, thoughts? Don't believe that it would take much effort.

Quoted 84 lines

Quoted 84 lines

runtest/syscalls +1 -0

index 2b8ca71..dfdc6cb 100644
@@ -537,6 +537,7 @@ fanotify09 fanotify09
 fanotify10 fanotify10
 fanotify11 fanotify11
 fanotify12 fanotify12
+fanotify13 fanotify13
 
 ioperm01 ioperm01
 ioperm02 ioperm02

testcases/kernel/syscalls/fanotify/.gitignore +1 -0

index 4256b8c..16bdd99 100644
@@ -10,4 +10,5 @@
 /fanotify10
 /fanotify11
 /fanotify12
+/fanotify13
 /fanotify_child

testcases/kernel/syscalls/fanotify/fanotify.h +37 -3

index 14654b7..e9b23cc 100644
@@ -29,6 +29,11 @@
 #define	__FANOTIFY_H__
 
 #include "config.h"
+#include <sys/statfs.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <fcntl.h>
 
 #if defined(HAVE_SYS_FANOTIFY_H)
 

@@ -57,9 +62,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 +91,50 @@ 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];
+};
+
+/*
+ * Helper function used to obtain __kernel_fsid_t and file_handle objects
+ * for a given path. Used by test files correlated to FAN_REPORT_FID
+ * functionality.
+ */
+static inline void fanotify_get_fid(const char *path, __kernel_fsid_t *fsid,
+			struct file_handle *handle)
+{
+	int mount_id;
+	struct statfs stats;
+
+	if (statfs(path, &stats) == -1)
+		tst_brk(TBROK | TERRNO,
+			"statfs(%s, ...) failed", path);
+	memcpy(fsid, &stats.f_fsid, sizeof(stats.f_fsid));
+
+	if (name_to_handle_at(AT_FDCWD, path, handle, &mount_id, 0) == -1) {

Quoted 8 lines

testcases/kernel/syscalls/fanotify/fanotify.h struct fanotify_mark_type { · line 129

Quoted 4 lines

testcases/kernel/syscalls/fanotify/fanotify.h +4 -0

+		if (errno == EOPNOTSUPP) {
+			tst_res(TCONF,
+				"filesystem %s does not support file handles",
+				tst_device->fs_type);

Btw, here the tst_res() does not make much sense sice the code will continue and we will exit the test with the tst_brk() below. Shouldn't we use tst_brk() here as well?

Yes, we should be using tst_brk(...) instead. Thanks for picking this up.

Quoted 10 lines

Quoted 10 lines

testcases/kernel/syscalls/fanotify/fanotify.h +8 -0

+		}
+		tst_brk(TBROK | TERRNO,
+			"name_to_handle_at(AT_FDCWD, %s, ...) failed", path);
+	}
+}
+
+#endif
+
 #define INIT_FANOTIFY_MARK_TYPE(t) \
 	{ FAN_MARK_ ## t, "FAN_MARK_" #t }

Quoted 11 lines · Cyril Hrubis

Hi!
I've tried these tests on buildservice to check that there are no problems on slightly older distributions and found two.

The first one is that we do define fallback definitions in fanotify.h but then ifdef the whole test code in #if defined(HAVE_SYS_FANOTIFY_H) so that it's efectively disabled on older distros even with fallback definitions in place. Also it's TST_TEST_TCONF() but that is just easy to fix typo. I guess that we can remove the ifdef and sys/fanotify.h include from the test sources since we conditionally include the sys/fanotify.h in the local fanotify.h already.

OK. In that case I can write a patch that is to be applied prior to this series which essentially just removes this specific preprocessor conditional directive from all the source files. Would you like me to do this?

Quoted 7 lines

The second one is that we fail to compile on older distributions because of missing name_to_handle_at() so we need configure check for that syscall and fallback definition in lapi/ header, or at least configure check and ifdef in the fanotify_get_fid() function. Which should be as easy as adding a name_to_handle_at line to AC_CHECK_FUNCS() in the configure.ac and using the macro from config.h.

Sure. I've gone ahead an updated it to accommodate for this. Prior to submitting through another patches series, changes can be found here:
https://github.com/matthewbobrowski/ltp/commit/54264db0e574d2f90e716a510fcb1da11ee174dc.

I think we can do better and also provide a fallback definition though, thoughts? Don't believe that it would take much effort.

Quoted 13 lines · Cyril Hrubis

Quoted 11 lines

Hi!
I've tried these tests on buildservice to check that there are no problems on slightly older distributions and found two.

The first one is that we do define fallback definitions in fanotify.h but then ifdef the whole test code in #if defined(HAVE_SYS_FANOTIFY_H) so that it's efectively disabled on older distros even with fallback definitions in place. Also it's TST_TEST_TCONF() but that is just easy to fix typo. I guess that we can remove the ifdef and sys/fanotify.h include from the test sources since we conditionally include the sys/fanotify.h in the local fanotify.h already.

Quoted 3 lines

OK. In that case I can write a patch that is to be applied prior to this series which essentially just removes this specific preprocessor conditional directive from all the source files. Would you like me to do this?

If I understand the concern correctly, it is not really important to address now for these new tests that check functionality only available since kernel v5.1. It could be addressed by followup work.

Quoted 11 lines

Quoted 6 lines

The second one is that we fail to compile on older distributions because of missing name_to_handle_at() so we need configure check for that syscall and fallback definition in lapi/ header, or at least configure check and ifdef in the fanotify_get_fid() function. Which should be as easy as adding a name_to_handle_at line to AC_CHECK_FUNCS() in the configure.ac and using the macro from config.h.

Sure. I've gone ahead an updated it to accommodate for this. Prior to submitting through another patches series, changes can be found here:
https://github.com/matthewbobrowski/ltp/commit/54264db0e574d2f90e716a510fcb1da11ee174dc.

Quoted 2 lines

I think we can do better and also provide a fallback definition though, thoughts? Don't believe that it would take much effort.

No reason to do that. Those tests will only be supported on kernel >= v5.1 old distros are less interesting for these tests.

Please post the patches for fanotify_dirent_3 branch.