All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup
@ 2020-12-01 17:42 Petr Vorel
  2020-12-01 17:42 ` [LTP] [PATCH v5 01/10] fanotify12: Drop incorrect hint Petr Vorel
                   ` (11 more replies)
  0 siblings, 12 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-01 17:42 UTC (permalink / raw)
  To: ltp

Hi Amir,

Changes v4->v5:
* Fix issue with "fanotify: Add helper for FAN_REPORT_FID support on fs"
  on fanotify01 (unwanted skipping tests). FAN_REPORT_FID is now tested
  both for general support in kernel with fanotify_init() and support on
  tested filesystem in fanotify_mark().

* Fix issue with FAN_MARK_FILESYSTEM (new commit "fanotify: Add helper
  for mark support check").

* Split "[v4,2/6] fanotify: Handle supported features checks in setup()"
  into two commits:
  fanotify: Add helper for access permission check
  fanotify: Add helper for event support check
  (easier to review).

* Drop commit "[v4,5/6] fanotify: Check FAN_REPORT_{FID, NAME} support"
  => IMHO not needed now, as there are {REQUIRE_,}FANOTIFY_FAN_REPORT_FID_ERR_MSG()
  helpers which check for FAN_REPORT_FID. Or am I'm wrong and you need it
  for your patchset?
  There will be needed to add also helper for FAN_REPORT_NAME/FAN_REPORT_DIR_FID
  for fanotify10.c (for kernels > 5.1 && < 5.9), but this is now covered by check
  in create_fanotify_groups() and I don't want to block your patchset even more.
  And this helper should eliminate a need for check in safe_fanotify_init().

* Also safe_fanotify_mark() got simplified more (removing check for
  FAN_MARK_FILESYSTEM and EOPNOTSUPP etc.).

* Properly remove TCONF messages in commit where they're replaced
  (fanotify10.c, also for FAN_REPORT_FID in fanotify01.c and
  fanotify15.c due FAN_REPORT_FID handled in
  {REQUIRE_,}FANOTIFY_FAN_REPORT_FID_ERR_MSG()).

* New commits:
  fanotify: Add helper for mark support check
  fanotify: Use tst_brk_ in safe_fanotify_init()
  fanotify16: Test also on FUSE
  fanotify: Cleanup <sys/fanotify.h> use

I tested everything on kernel without FAN_MARK_FILESYSTEM and
FAN_REPORT_FID support and on newer kernels (5.3.18, 5.7.1, 5.10.0-rc5).

Hope I haven't omitted anything this time (apart from helper for
FAN_REPORT_NAME/FAN_REPORT_DIR_FID).

Kind regards,
Petr

Petr Vorel (10):
  fanotify12: Drop incorrect hint
  fanotify: Add helper for access permission check
  fanotify: Add helper for event support check
  fanotify: Add helper for FAN_REPORT_FID support on fs
  fanotify16: Test also on FUSE
  fanotify: Add helper for mark support check
  fanotify: Introduce SAFE_FANOTIFY_MARK() macro
  fanotify: Use tst_brk_ in safe_fanotify_init()
  fanotify: Add a pedantic check for return value
  fanotify: Cleanup <sys/fanotify.h> use

 testcases/kernel/syscalls/fanotify/fanotify.h | 191 +++++++++++++++---
 .../kernel/syscalls/fanotify/fanotify01.c     |  82 +++-----
 .../kernel/syscalls/fanotify/fanotify02.c     |  27 +--
 .../kernel/syscalls/fanotify/fanotify03.c     |  66 ++----
 .../kernel/syscalls/fanotify/fanotify04.c     |  37 +---
 .../kernel/syscalls/fanotify/fanotify05.c     |  14 +-
 .../kernel/syscalls/fanotify/fanotify06.c     |  26 +--
 .../kernel/syscalls/fanotify/fanotify07.c     |  22 +-
 .../kernel/syscalls/fanotify/fanotify08.c     |   5 +-
 .../kernel/syscalls/fanotify/fanotify09.c     |  24 +--
 .../kernel/syscalls/fanotify/fanotify10.c     |  56 ++---
 .../kernel/syscalls/fanotify/fanotify11.c     |  12 +-
 .../kernel/syscalls/fanotify/fanotify12.c     |  63 ++----
 .../kernel/syscalls/fanotify/fanotify13.c     |  59 ++----
 .../kernel/syscalls/fanotify/fanotify14.c     |   6 +-
 .../kernel/syscalls/fanotify/fanotify15.c     |  47 +----
 .../kernel/syscalls/fanotify/fanotify16.c     |  32 +--
 17 files changed, 320 insertions(+), 449 deletions(-)

-- 
2.29.2


^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 01/10] fanotify12: Drop incorrect hint
  2020-12-01 17:42 [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
@ 2020-12-01 17:42 ` Petr Vorel
  2020-12-02 15:09   ` Cyril Hrubis
  2020-12-01 17:42 ` [LTP] [PATCH v5 02/10] fanotify: Add helper for access permission check Petr Vorel
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Petr Vorel @ 2020-12-01 17:42 UTC (permalink / raw)
  To: ltp

hint "CONFIG_FANOTIFY_ACCESS_PERMISSIONS not configured in kernel?"
is wrong here (fanotify12 does not use FAN_ACCESS_PERM).

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Same as v4.

 testcases/kernel/syscalls/fanotify/fanotify12.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
index fcb7ec0d3..fea7cb607 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify12.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
@@ -146,10 +146,6 @@ static int setup_mark(unsigned int n)
 					"FAN_OPEN_EXEC not supported in "
 					"kernel?");
 				return -1;
-			} else if (errno == EINVAL) {
-				tst_brk(TCONF | TERRNO,
-					"CONFIG_FANOTIFY_ACCESS_PERMISSIONS "
-					"not configured in kernel?");
 			}else {
 				tst_brk(TBROK | TERRNO,
 					"fanotify_mark(%d, FAN_MARK_ADD | %s, "
@@ -173,11 +169,6 @@ static int setup_mark(unsigned int n)
 						"FAN_OPEN_EXEC not supported "
 						"in kernel?");
 					return -1;
-				} else if (errno == EINVAL) {
-					tst_brk(TCONF | TERRNO,
-						"CONFIG_FANOTIFY_ACCESS_"
-						"PERMISSIONS not configured in "
-						"kernel?");
 				} else {
 					tst_brk(TBROK | TERRNO,
 						"fanotify_mark (%d, "
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 02/10] fanotify: Add helper for access permission check
  2020-12-01 17:42 [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
  2020-12-01 17:42 ` [LTP] [PATCH v5 01/10] fanotify12: Drop incorrect hint Petr Vorel
@ 2020-12-01 17:42 ` Petr Vorel
  2020-12-02 15:24   ` Cyril Hrubis
  2020-12-01 17:42 ` [LTP] [PATCH v5 03/10] fanotify: Add helper for event support check Petr Vorel
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Petr Vorel @ 2020-12-01 17:42 UTC (permalink / raw)
  To: ltp

i.e. CONFIG_FANOTIFY_ACCESS_PERMISSIONS
and use it in relevant tests setup().

The purpose is to reduce checks (cleanup).
Permission is required by all tests cases, hence tst_brk().

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Originally part of "[v4,2/6] fanotify: Handle supported features checks in setup()".

 testcases/kernel/syscalls/fanotify/fanotify.h | 19 +++++++++++++++++++
 .../kernel/syscalls/fanotify/fanotify03.c     |  2 ++
 .../kernel/syscalls/fanotify/fanotify07.c     | 14 +++++---------
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 0afbc02aa..687089ace 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -242,4 +242,23 @@ static inline void fanotify_save_fid(const char *path,
 #define INIT_FANOTIFY_MARK_TYPE(t) \
 	{ FAN_MARK_ ## t, "FAN_MARK_" #t }
 
+static inline void require_fanotify_access_permissions_supported_by_kernel(void)
+{
+	int fd;
+
+	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
+
+	if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, ".") < 0) {
+		if (errno == EINVAL) {
+			tst_brk(TCONF | TERRNO,
+				"CONFIG_FANOTIFY_ACCESS_PERMISSIONS not configured in kernel?");
+		} else {
+			tst_brk(TBROK | TERRNO,
+				"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, \".\") failed", fd);
+		}
+	}
+
+	SAFE_CLOSE(fd);
+}
+
 #endif /* __FANOTIFY_H__ */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
index 1ef1c206b..cd5576f3a 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify03.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
@@ -347,6 +347,8 @@ static void test_fanotify(unsigned int n)
 
 static void setup(void)
 {
+	require_fanotify_access_permissions_supported_by_kernel();
+
 	sprintf(fname, MOUNT_PATH"/fname_%d", getpid());
 	SAFE_FILE_PRINTF(fname, "1");
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify07.c b/testcases/kernel/syscalls/fanotify/fanotify07.c
index c2e185710..8822e9c51 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify07.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify07.c
@@ -106,15 +106,9 @@ static int setup_instance(void)
 	if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD,
 			  fname) < 0) {
 		close(fd);
-		if (errno == EINVAL) {
-			tst_brk(TCONF | TERRNO,
-				"CONFIG_FANOTIFY_ACCESS_PERMISSIONS not "
-				"configured in kernel?");
-		} else {
-			tst_brk(TBROK | TERRNO,
-				"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS_PERM, "
-				"AT_FDCWD, %s) failed.", fd, fname);
-		}
+		tst_brk(TBROK | TERRNO,
+			"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, %s) failed",
+			fd, fname);
 	}
 
 	return fd;
@@ -195,6 +189,8 @@ static void test_fanotify(void)
 
 static void setup(void)
 {
+	require_fanotify_access_permissions_supported_by_kernel();
+
 	sprintf(fname, "fname_%d", getpid());
 	SAFE_FILE_PRINTF(fname, "%s", fname);
 }
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 03/10] fanotify: Add helper for event support check
  2020-12-01 17:42 [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
  2020-12-01 17:42 ` [LTP] [PATCH v5 01/10] fanotify12: Drop incorrect hint Petr Vorel
  2020-12-01 17:42 ` [LTP] [PATCH v5 02/10] fanotify: Add helper for access permission check Petr Vorel
@ 2020-12-01 17:42 ` Petr Vorel
  2020-12-02 15:36   ` Cyril Hrubis
  2020-12-01 17:42 ` [LTP] [PATCH v5 04/10] fanotify: Add helper for FAN_REPORT_FID support on fs Petr Vorel
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Petr Vorel @ 2020-12-01 17:42 UTC (permalink / raw)
  To: ltp

i.e. FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM
and use it in relevant tests setup().

The purpose is to reduce checks (cleanup).

Due this change test with FAN_OPEN_EXEC_PERM in fanotify03.c no longer
needs to be a first test.

Also rename variable s/support_exec_events/exec_events_unsupported/
for readability.

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Originally part of "[v4,2/6] fanotify: Handle supported features checks in setup()"
(but fixed name - drop "exec").

 testcases/kernel/syscalls/fanotify/fanotify.h | 22 ++++++++
 .../kernel/syscalls/fanotify/fanotify03.c     | 34 ++++--------
 .../kernel/syscalls/fanotify/fanotify10.c     | 15 +++---
 .../kernel/syscalls/fanotify/fanotify12.c     | 53 +++++++++----------
 4 files changed, 63 insertions(+), 61 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 687089ace..8c677643f 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -60,6 +60,7 @@ int safe_fanotify_init(const char *file, const int lineno,
 
 	return rval;
 }
+
 #define SAFE_FANOTIFY_INIT(fan, mode)  \
 	safe_fanotify_init(__FILE__, __LINE__, (fan), (mode))
 
@@ -261,4 +262,25 @@ static inline void require_fanotify_access_permissions_supported_by_kernel(void)
 	SAFE_CLOSE(fd);
 }
 
+static inline int fanotify_events_supported_by_kernel(uint64_t mask)
+{
+	int fd;
+	int rval = 0;
+
+	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
+
+	if (fanotify_mark(fd, FAN_MARK_ADD, mask, AT_FDCWD, ".") < 0) {
+		if (errno == EINVAL) {
+			rval = -1;
+		} else {
+			tst_brk(TBROK | TERRNO,
+				"fanotify_mark (%d, FAN_MARK_ADD, ..., AT_FDCWD, \".\") failed", fd);
+		}
+	}
+
+	SAFE_CLOSE(fd);
+
+	return rval;
+}
+
 #endif /* __FANOTIFY_H__ */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
index cd5576f3a..75e5b852d 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify03.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
@@ -48,18 +48,13 @@ static volatile int fd_notify;
 static pid_t child_pid;
 
 static char event_buf[EVENT_BUF_LEN];
-static int support_exec_events;
+static int exec_events_unsupported;
 
 struct event {
 	unsigned long long mask;
 	unsigned int response;
 };
 
-/*
- * Ensure to keep the first FAN_OPEN_EXEC_PERM test case before the first
- * MARK_TYPE(FILESYSTEM) in order to allow for correct detection between
- * exec events not supported and filesystem marks not supported.
- */
 static struct tcase {
 	const char *tname;
 	struct fanotify_mark_type mark;
@@ -212,28 +207,23 @@ static int setup_mark(unsigned int n)
 	char *const files[] = {fname, FILE_EXEC_PATH};
 
 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
+
+	if (exec_events_unsupported && tc->mask & FAN_OPEN_EXEC_PERM) {
+		tst_res(TCONF, "FAN_OPEN_EXEC_PERM not supported in kernel?");
+		return -1;
+	}
+
 	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
 
 	for (; i < ARRAY_SIZE(files); i++) {
 		if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
 				  tc->mask, AT_FDCWD, files[i]) < 0) {
 			if (errno == EINVAL &&
-				(tc->mask & FAN_OPEN_EXEC_PERM &&
-				 !support_exec_events)) {
-				tst_res(TCONF,
-					"FAN_OPEN_EXEC_PERM not supported in "
-					"kernel?");
-				return -1;
-			} else if (errno == EINVAL &&
 					mark->flag == FAN_MARK_FILESYSTEM) {
 				tst_res(TCONF,
 					"FAN_MARK_FILESYSTEM not supported in "
 					"kernel?");
 				return -1;
-			} else if (errno == EINVAL) {
-				tst_brk(TCONF | TERRNO,
-					"CONFIG_FANOTIFY_ACCESS_PERMISSIONS "
-					"not configured in kernel?");
 			} else {
 				tst_brk(TBROK | TERRNO,
 					"fanotify_mark(%d, FAN_MARK_ADD | %s, "
@@ -241,14 +231,6 @@ static int setup_mark(unsigned int n)
 					"AT_FDCWD, %s) failed.",
 					fd_notify, mark->name, fname);
 			}
-		} else {
-			/*
-			 * To distinguish between perm not supported, exec
-			 * events not supported and filesystem mark not
-			 * supported.
-			 */
-			if (tc->mask & FAN_OPEN_EXEC_PERM)
-				support_exec_events = 1;
 		}
 	}
 
@@ -349,6 +331,8 @@ static void setup(void)
 {
 	require_fanotify_access_permissions_supported_by_kernel();
 
+	exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC_PERM);
+
 	sprintf(fname, MOUNT_PATH"/fname_%d", getpid());
 	SAFE_FILE_PRINTF(fname, "1");
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index 90cf5cb5f..73702285c 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -64,6 +64,7 @@ static unsigned int fanotify_class[] = {
 static int fd_notify[NUM_CLASSES][GROUPS_PER_PRIO];
 
 static char event_buf[EVENT_BUF_LEN];
+static int exec_events_unsupported;
 
 #define MOUNT_PATH "fs_mnt"
 #define MNT2_PATH "mntpoint"
@@ -323,13 +324,6 @@ static int create_fanotify_groups(unsigned int n)
 					    AT_FDCWD, tc->mark_path);
 			if (ret < 0) {
 				if (errno == EINVAL &&
-				    tc->expected_mask_without_ignore &
-				    FAN_OPEN_EXEC) {
-					tst_res(TCONF,
-						"FAN_OPEN_EXEC not supported "
-						"by kernel?");
-					return -1;
-				} else if (errno == EINVAL &&
 					tc->mark_type == FANOTIFY_FILESYSTEM) {
 					tst_res(TCONF,
 						"FAN_MARK_FILESYSTEM not "
@@ -451,6 +445,11 @@ static void test_fanotify(unsigned int n)
 
 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
 
+	if (exec_events_unsupported && tc->expected_mask_with_ignore & FAN_OPEN_EXEC) {
+		tst_res(TCONF, "FAN_OPEN_EXEC not supported in kernel?");
+		return;
+	}
+
 	if (tc->ignored_onchild && tst_kvercmp(5, 9, 0) < 0) {
 		tst_res(TCONF, "ignored mask in combination with flag FAN_EVENT_ON_CHILD"
 				" has undefined behavior on kernel < 5.9");
@@ -535,6 +534,8 @@ cleanup:
 
 static void setup(void)
 {
+	exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC);
+
 	/* Create another bind mount@another path for generating events */
 	SAFE_MKDIR(MNT2_PATH, 0755);
 	SAFE_MOUNT(MOUNT_PATH, MNT2_PATH, "none", MS_BIND, NULL);
diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
index fea7cb607..0d049fc6b 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify12.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
@@ -39,6 +39,7 @@ static char fname[BUF_SIZE];
 static volatile int fd_notify;
 static volatile int complete;
 static char event_buf[EVENT_BUF_LEN];
+static int exec_events_unsupported;
 
 static struct test_case_t {
 	const char *tname;
@@ -135,26 +136,26 @@ static int setup_mark(unsigned int n)
 	const char *const files[] = {fname, TEST_APP};
 
 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
+
+	if (exec_events_unsupported && ((tc->mask & FAN_OPEN_EXEC) ||
+					tc->ignore_mask & FAN_OPEN_EXEC)) {
+		tst_res(TCONF, "FAN_OPEN_EXEC not supported in kernel?");
+		return -1;
+	}
+
 	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
 
 	for (; i < ARRAY_SIZE(files); i++) {
 		/* Setup normal mark on object */
 		if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
 					tc->mask, AT_FDCWD, files[i]) < 0) {
-			if (errno == EINVAL && tc->mask & FAN_OPEN_EXEC) {
-				tst_res(TCONF,
-					"FAN_OPEN_EXEC not supported in "
-					"kernel?");
-				return -1;
-			}else {
-				tst_brk(TBROK | TERRNO,
-					"fanotify_mark(%d, FAN_MARK_ADD | %s, "
-					"%llx, AT_FDCWD, %s) failed",
-					fd_notify,
-					mark->name,
-					tc->mask,
-					files[i]);
-			}
+			tst_brk(TBROK | TERRNO,
+				"fanotify_mark(%d, FAN_MARK_ADD | %s, "
+				"%llx, AT_FDCWD, %s) failed",
+				fd_notify,
+				mark->name,
+				tc->mask,
+				files[i]);
 		}
 
 		/* Setup ignore mark on object */
@@ -163,21 +164,13 @@ static int setup_mark(unsigned int n)
 						| FAN_MARK_IGNORED_MASK,
 						tc->ignore_mask, AT_FDCWD,
 						files[i]) < 0) {
-				if (errno == EINVAL &&
-					tc->ignore_mask & FAN_OPEN_EXEC) {
-					tst_res(TCONF,
-						"FAN_OPEN_EXEC not supported "
-						"in kernel?");
-					return -1;
-				} else {
-					tst_brk(TBROK | TERRNO,
-						"fanotify_mark (%d, "
-						"FAN_MARK_ADD | %s "
-						"| FAN_MARK_IGNORED_MASK, "
-						"%llx, AT_FDCWD, %s) failed",
-						fd_notify, mark->name,
-						tc->ignore_mask, files[i]);
-				}
+				tst_brk(TBROK | TERRNO,
+					"fanotify_mark (%d, "
+					"FAN_MARK_ADD | %s "
+					"| FAN_MARK_IGNORED_MASK, "
+					"%llx, AT_FDCWD, %s) failed",
+					fd_notify, mark->name,
+					tc->ignore_mask, files[i]);
 			}
 		}
 	}
@@ -244,6 +237,8 @@ cleanup:
 
 static void do_setup(void)
 {
+	exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC);
+
 	sprintf(fname, "fname_%d", getpid());
 	SAFE_FILE_PRINTF(fname, "1");
 }
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 04/10] fanotify: Add helper for FAN_REPORT_FID support on fs
  2020-12-01 17:42 [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
                   ` (2 preceding siblings ...)
  2020-12-01 17:42 ` [LTP] [PATCH v5 03/10] fanotify: Add helper for event support check Petr Vorel
@ 2020-12-01 17:42 ` Petr Vorel
  2020-12-02 15:46   ` Cyril Hrubis
  2020-12-01 17:42 ` [LTP] [PATCH v5 05/10] fanotify16: Test also on FUSE Petr Vorel
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Petr Vorel @ 2020-12-01 17:42 UTC (permalink / raw)
  To: ltp

and use it in relevant tests setup().
The purpose is to reduce checks (cleanup).

FAN_REPORT_FID can be unsupported in kernel entirely (detected in
fanotify_init()) or just missing for particular fs (detected in
fanotify_mark().

The check is related to kernel fix
a8b13aa20afb ("fanotify: enable FAN_REPORT_FID init flag")

NOTE: FAN_REPORT_FID is missing (at least) on exfat and ntfs over FUSE.
Original motivation for cleanup in this and previous commits was not
only to simplify the code but also check for unsupported filesystems
which was missing least for exfat on fanotify16, because filesystem can
be set via LTP_DEV_FS_TYPE environment variable on tests which don't
tests all filesystems.

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v4->v5:
* Fix issue with "fanotify: Add helper for FAN_REPORT_FID support on fs"
  on fanotify01 (unwanted skipping tests). FAN_REPORT_FID is now tested
  both for general support in kernel with fanotify_init() and support on
  tested filesystem in fanotify_mark().

 testcases/kernel/syscalls/fanotify/fanotify.h | 68 +++++++++++++++++++
 .../kernel/syscalls/fanotify/fanotify01.c     | 19 +++---
 .../kernel/syscalls/fanotify/fanotify13.c     | 23 +------
 .../kernel/syscalls/fanotify/fanotify15.c     | 21 +-----
 .../kernel/syscalls/fanotify/fanotify16.c     |  6 +-
 5 files changed, 82 insertions(+), 55 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 8c677643f..821e6cb29 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -283,4 +283,72 @@ static inline int fanotify_events_supported_by_kernel(uint64_t mask)
 	return rval;
 }
 
+/*
+ * @return  0: fanotify supported both in kernel and on tested filesystem
+ * @return -1: FAN_REPORT_FID not supported in kernel
+ * @return -2: FAN_REPORT_FID not supported on tested filesystem
+ */
+static inline int fanotify_fan_report_fid_supported_on_fs(const char *fname)
+{
+	int fd;
+	int rval = 0;
+
+	fd = fanotify_init(FAN_CLASS_NOTIF | FAN_REPORT_FID, O_RDONLY);
+
+	if (fd < 0) {
+		if (errno == ENOSYS)
+			tst_brk(TCONF, "fanotify not configured in kernel");
+
+		if (errno == EINVAL)
+			return -1;
+
+		tst_brk(TBROK | TERRNO, "fanotify_init() failed");
+	}
+
+	if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS, AT_FDCWD, fname) < 0) {
+		if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV) {
+			rval = -2;
+		} else {
+			tst_brk(TBROK | TERRNO,
+				"fanotify_mark (%d, FAN_MARK_ADD, ..., AT_FDCWD, \".\") failed", fd);
+		}
+	}
+
+	SAFE_CLOSE(fd);
+
+	return rval;
+}
+
+typedef void (*tst_res_func_t)(const char *file, const int lineno,
+		int ttype, const char *fmt, ...);
+
+static inline void fanotify_fan_report_fid_err_msg(const char *file,
+	const int lineno, tst_res_func_t res_func, int fail)
+{
+	if (fail == -1)
+		res_func(file, lineno, TCONF,
+			 "FAN_REPORT_FID not supported in kernel?");
+
+	if (fail == -2)
+		res_func(file, lineno, TCONF,
+			 "FAN_REPORT_FID not supported on %s filesystem",
+			 tst_device->fs_type);
+}
+
+#define FANOTIFY_FAN_REPORT_FID_ERR_MSG(fail) \
+	fanotify_fan_report_fid_err_msg(__FILE__, __LINE__, tst_res_, (fail))
+
+static inline void require_fanotify_fan_report_fid_supported_on_fs(const char *file,
+	const int lineno, const char *fname)
+{
+	int rval;
+
+	rval = fanotify_fan_report_fid_supported_on_fs(fname);
+	if (rval)
+		fanotify_fan_report_fid_err_msg(file, lineno, tst_brk_, rval);
+}
+
+#define REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(fname) \
+	require_fanotify_fan_report_fid_supported_on_fs(__FILE__, __LINE__, (fname))
+
 #endif /* __FANOTIFY_H__ */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
index 03e453f41..5f2544931 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify01.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
@@ -74,6 +74,7 @@ static struct tcase {
 static char fname[BUF_SIZE];
 static char buf[BUF_SIZE];
 static int fd_notify;
+static int fan_report_fid_unsupported;
 
 static unsigned long long event_set[EVENT_MAX];
 
@@ -88,19 +89,13 @@ static void test_fanotify(unsigned int n)
 
 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
 
-	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 (fan_report_fid_unsupported && (tc->init_flags & FAN_REPORT_FID)) {
+		FANOTIFY_FAN_REPORT_FID_ERR_MSG(fan_report_fid_unsupported);
+		return;
 	}
 
+	fd_notify = SAFE_FANOTIFY_INIT(tc->init_flags, O_RDONLY);
+
 	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
 			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
 			  AT_FDCWD, fname) < 0) {
@@ -366,6 +361,8 @@ static void setup(void)
 
 	sprintf(fname, MOUNT_PATH"/tfile_%d", getpid());
 	SAFE_FILE_PRINTF(fname, "1");
+
+	fan_report_fid_unsupported = fanotify_fan_report_fid_supported_on_fs(fname);
 }
 
 static void cleanup(void)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index c2a21bb66..81fe02db2 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -124,14 +124,6 @@ static int setup_marks(unsigned int fd, struct test_case_t *tc)
 					"FAN_MARK_FILESYSTEM not supported by "
 					"kernel");
 				return 1;
-			} else if (errno == ENODEV &&
-				   !objects[i].fid.fsid.val[0] &&
-				   !objects[i].fid.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, "
@@ -162,17 +154,7 @@ static void do_test(unsigned int number)
 		"Test #%d: FAN_REPORT_FID with mark flag: %s",
 		number, mark->name);
 
-	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");
-	}
+	fanotify_fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | FAN_REPORT_FID, O_RDONLY);
 
 	/*
 	 * Place marks on a set of objects and setup the expected masks
@@ -281,7 +263,8 @@ out:
 
 static void do_setup(void)
 {
-	/* Check for kernel fanotify support */
+	REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(MOUNT_PATH);
+
 	nofid_fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
 
 	/* Create file and directory objects for testing */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify15.c b/testcases/kernel/syscalls/fanotify/fanotify15.c
index d787a08e3..2e3044ad7 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify15.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify15.c
@@ -89,10 +89,6 @@ static void do_test(unsigned int number)
 				FAN_CREATE | FAN_DELETE | FAN_MOVE |
 				FAN_MODIFY | FAN_ONDIR,
 				AT_FDCWD, TEST_DIR) == -1) {
-		if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV)
-			tst_brk(TCONF,
-				"FAN_REPORT_FID not supported on %s "
-				"filesystem", tst_device->fs_type);
 		tst_brk(TBROK | TERRNO,
 			"fanotify_mark(%d, FAN_MARK_ADD | %s, "
 			"FAN_CREATE | FAN_DELETE | FAN_MOVE | "
@@ -287,22 +283,9 @@ static void do_test(unsigned int number)
 
 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);
+	REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(TEST_DIR);
+	fanotify_fd = SAFE_FANOTIFY_INIT(FAN_REPORT_FID, O_RDONLY);
 }
 
 static void do_cleanup(void)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
index 7995a1688..c02a57dcd 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify16.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify16.c
@@ -562,11 +562,7 @@ check_match:
 
 static void setup(void)
 {
-	int fd;
-
-	/* Check kernel for fanotify support */
-	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
-	SAFE_CLOSE(fd);
+	REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(MOUNT_PATH);
 
 	sprintf(dname1, "%s/%s", MOUNT_PATH, DIR_NAME1);
 	sprintf(dname2, "%s/%s", MOUNT_PATH, DIR_NAME2);
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 05/10] fanotify16: Test also on FUSE
  2020-12-01 17:42 [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
                   ` (3 preceding siblings ...)
  2020-12-01 17:42 ` [LTP] [PATCH v5 04/10] fanotify: Add helper for FAN_REPORT_FID support on fs Petr Vorel
@ 2020-12-01 17:42 ` Petr Vorel
  2020-12-02 15:46   ` Cyril Hrubis
  2020-12-01 17:42 ` [LTP] [PATCH v5 06/10] fanotify: Add helper for mark support check Petr Vorel
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Petr Vorel @ 2020-12-01 17:42 UTC (permalink / raw)
  To: ltp

.dev_fs_flags = TST_FS_SKIP_FUSE flag was requested due missing
FAN_REPORT_FID support. But that's now covered (in previous commit).

It's better to not cast the filesystem when not needed (it might gain
the support later) + unify with other tests (fanotify13.c).

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v5.

 testcases/kernel/syscalls/fanotify/fanotify16.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
index c02a57dcd..a379226f8 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify16.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify16.c
@@ -579,7 +579,6 @@ static void cleanup(void)
 static struct tst_test test = {
 	.test = do_test,
 	.tcnt = ARRAY_SIZE(test_cases),
-	.dev_fs_flags = TST_FS_SKIP_FUSE,
 	.setup = setup,
 	.cleanup = cleanup,
 	.mount_device = 1,
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 06/10] fanotify: Add helper for mark support check
  2020-12-01 17:42 [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
                   ` (4 preceding siblings ...)
  2020-12-01 17:42 ` [LTP] [PATCH v5 05/10] fanotify16: Test also on FUSE Petr Vorel
@ 2020-12-01 17:42 ` Petr Vorel
  2020-12-02  6:27   ` Amir Goldstein
  2020-12-02 15:53   ` Cyril Hrubis
  2020-12-01 17:42 ` [LTP] [PATCH v5 07/10] fanotify: Introduce SAFE_FANOTIFY_MARK() macro Petr Vorel
                   ` (5 subsequent siblings)
  11 siblings, 2 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-01 17:42 UTC (permalink / raw)
  To: ltp

i.e. FAN_MARK_FILESYSTEM and use it in relevant tests setup().
The purpose is to reduce checks (cleanup).

NOTE: all tests check only for FAN_MARK_FILESYSTEM, but keep helper
generic for future use.

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v5.

 testcases/kernel/syscalls/fanotify/fanotify.h | 21 ++++++++++++++++
 .../kernel/syscalls/fanotify/fanotify01.c     | 12 +++++----
 .../kernel/syscalls/fanotify/fanotify03.c     | 25 +++++++++----------
 .../kernel/syscalls/fanotify/fanotify10.c     | 14 +++++------
 .../kernel/syscalls/fanotify/fanotify13.c     | 15 +++++------
 5 files changed, 55 insertions(+), 32 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 821e6cb29..2275a1da3 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -351,4 +351,25 @@ static inline void require_fanotify_fan_report_fid_supported_on_fs(const char *f
 #define REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(fname) \
 	require_fanotify_fan_report_fid_supported_on_fs(__FILE__, __LINE__, (fname))
 
+static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
+{
+	int fd;
+	int rval = 0;
+
+	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
+
+	if (fanotify_mark(fd, FAN_MARK_ADD | flag, FAN_ACCESS_PERM, AT_FDCWD, ".") < 0) {
+		if (errno == EINVAL) {
+			rval = -1;
+		} else {
+			tst_brk(TBROK | TERRNO,
+				"fanotify_mark (%d, FAN_MARK_ADD, ..., FAN_ACCESS_PERM, AT_FDCWD, \".\") failed", fd);
+		}
+	}
+
+	SAFE_CLOSE(fd);
+
+	return rval;
+}
+
 #endif /* __FANOTIFY_H__ */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
index 5f2544931..c8ab41695 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify01.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
@@ -75,6 +75,7 @@ static char fname[BUF_SIZE];
 static char buf[BUF_SIZE];
 static int fd_notify;
 static int fan_report_fid_unsupported;
+static int filesystem_mark_unsupported;
 
 static unsigned long long event_set[EVENT_MAX];
 
@@ -94,16 +95,16 @@ static void test_fanotify(unsigned int n)
 		return;
 	}
 
+	if (filesystem_mark_unsupported && mark->flag == FAN_MARK_FILESYSTEM) {
+		tst_res(TCONF, "FAN_MARK_FILESYSTEM not supported in kernel?");
+		return;
+	}
+
 	fd_notify = SAFE_FANOTIFY_INIT(tc->init_flags, O_RDONLY);
 
 	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
 			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
 			  AT_FDCWD, fname) < 0) {
-		if (errno == EINVAL && mark->flag == FAN_MARK_FILESYSTEM) {
-			tst_res(TCONF,
-				"FAN_MARK_FILESYSTEM not supported in kernel?");
-			return;
-		}
 		tst_brk(TBROK | TERRNO,
 			"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS | %s | "
 			"FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, %s) "
@@ -363,6 +364,7 @@ static void setup(void)
 	SAFE_FILE_PRINTF(fname, "1");
 
 	fan_report_fid_unsupported = fanotify_fan_report_fid_supported_on_fs(fname);
+	filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
 }
 
 static void cleanup(void)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
index 75e5b852d..8b54e42c4 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify03.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
@@ -49,6 +49,7 @@ static pid_t child_pid;
 
 static char event_buf[EVENT_BUF_LEN];
 static int exec_events_unsupported;
+static int filesystem_mark_unsupported;
 
 struct event {
 	unsigned long long mask;
@@ -213,24 +214,21 @@ static int setup_mark(unsigned int n)
 		return -1;
 	}
 
+	if (filesystem_mark_unsupported && mark->flag == FAN_MARK_FILESYSTEM) {
+		tst_res(TCONF, "FAN_MARK_FILESYSTEM not supported in kernel?");
+		return -1;
+	}
+
 	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
 
 	for (; i < ARRAY_SIZE(files); i++) {
 		if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
 				  tc->mask, AT_FDCWD, files[i]) < 0) {
-			if (errno == EINVAL &&
-					mark->flag == FAN_MARK_FILESYSTEM) {
-				tst_res(TCONF,
-					"FAN_MARK_FILESYSTEM not supported in "
-					"kernel?");
-				return -1;
-			} else {
-				tst_brk(TBROK | TERRNO,
-					"fanotify_mark(%d, FAN_MARK_ADD | %s, "
-					"FAN_ACCESS_PERM | FAN_OPEN_PERM, "
-					"AT_FDCWD, %s) failed.",
-					fd_notify, mark->name, fname);
-			}
+			tst_brk(TBROK | TERRNO,
+				"fanotify_mark(%d, FAN_MARK_ADD | %s, "
+				"FAN_ACCESS_PERM | FAN_OPEN_PERM, "
+				"AT_FDCWD, %s) failed.",
+				fd_notify, mark->name, fname);
 		}
 	}
 
@@ -331,6 +329,7 @@ static void setup(void)
 {
 	require_fanotify_access_permissions_supported_by_kernel();
 
+	filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
 	exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC_PERM);
 
 	sprintf(fname, MOUNT_PATH"/fname_%d", getpid());
diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index 73702285c..6d048958c 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -65,6 +65,7 @@ static int fd_notify[NUM_CLASSES][GROUPS_PER_PRIO];
 
 static char event_buf[EVENT_BUF_LEN];
 static int exec_events_unsupported;
+static int filesystem_mark_unsupported;
 
 #define MOUNT_PATH "fs_mnt"
 #define MNT2_PATH "mntpoint"
@@ -323,13 +324,6 @@ static int create_fanotify_groups(unsigned int n)
 					    FAN_EVENT_ON_CHILD,
 					    AT_FDCWD, tc->mark_path);
 			if (ret < 0) {
-				if (errno == EINVAL &&
-					tc->mark_type == FANOTIFY_FILESYSTEM) {
-					tst_res(TCONF,
-						"FAN_MARK_FILESYSTEM not "
-						"supported in kernel?");
-					return -1;
-				}
 				tst_brk(TBROK | TERRNO,
 					"fanotify_mark(%d, FAN_MARK_ADD | %s,"
 					"FAN_OPEN, AT_FDCWD, %s) failed",
@@ -450,6 +444,11 @@ static void test_fanotify(unsigned int n)
 		return;
 	}
 
+	if (filesystem_mark_unsupported && tc->mark_type == FANOTIFY_FILESYSTEM) {
+		tst_res(TCONF, "FAN_MARK_FILESYSTEM not supported in kernel?");
+		return;
+	}
+
 	if (tc->ignored_onchild && tst_kvercmp(5, 9, 0) < 0) {
 		tst_res(TCONF, "ignored mask in combination with flag FAN_EVENT_ON_CHILD"
 				" has undefined behavior on kernel < 5.9");
@@ -535,6 +534,7 @@ cleanup:
 static void setup(void)
 {
 	exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC);
+	filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
 
 	/* Create another bind mount@another path for generating events */
 	SAFE_MKDIR(MNT2_PATH, 0755);
diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index 81fe02db2..33989d902 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -88,6 +88,7 @@ static struct test_case_t {
 
 static int nofid_fd;
 static int fanotify_fd;
+static int filesystem_mark_unsupported;
 static char events_buf[BUF_SIZE];
 static struct event_t event_set[EVENT_MAX];
 
@@ -118,13 +119,6 @@ static int setup_marks(unsigned int fd, struct test_case_t *tc)
 	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;
-			}
 			tst_brk(TBROK | TERRNO,
 				"fanotify_mark(%d, FAN_MARK_ADD, FAN_OPEN, "
 				"AT_FDCWD, %s) failed",
@@ -154,6 +148,11 @@ static void do_test(unsigned int number)
 		"Test #%d: FAN_REPORT_FID with mark flag: %s",
 		number, mark->name);
 
+	if (filesystem_mark_unsupported && mark->flag & FAN_MARK_FILESYSTEM) {
+		tst_res(TCONF, "FAN_MARK_FILESYSTEM not supported in kernel?");
+		return;
+	}
+
 	fanotify_fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | FAN_REPORT_FID, O_RDONLY);
 
 	/*
@@ -265,6 +264,8 @@ static void do_setup(void)
 {
 	REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(MOUNT_PATH);
 
+	filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
+
 	nofid_fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
 
 	/* Create file and directory objects for testing */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 07/10] fanotify: Introduce SAFE_FANOTIFY_MARK() macro
  2020-12-01 17:42 [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
                   ` (5 preceding siblings ...)
  2020-12-01 17:42 ` [LTP] [PATCH v5 06/10] fanotify: Add helper for mark support check Petr Vorel
@ 2020-12-01 17:42 ` Petr Vorel
  2020-12-02 15:55   ` Cyril Hrubis
  2020-12-01 17:42 ` [LTP] [PATCH v5 08/10] fanotify: Use tst_brk_ in safe_fanotify_init() Petr Vorel
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Petr Vorel @ 2020-12-01 17:42 UTC (permalink / raw)
  To: ltp

and safe_fanotify_mark() function and use it in all tests which use
fanotify_mark().

Simple checking in safe_fanotify_mark() was possible due helpers added
in previous commits which are used in tests' setup functions.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v4->v5:
* Less cleanup here, because many TCONF messages were removed earlier.

 testcases/kernel/syscalls/fanotify/fanotify.h | 23 +++++++++
 .../kernel/syscalls/fanotify/fanotify01.c     | 48 ++++---------------
 .../kernel/syscalls/fanotify/fanotify02.c     | 22 ++-------
 .../kernel/syscalls/fanotify/fanotify03.c     | 10 +---
 .../kernel/syscalls/fanotify/fanotify04.c     | 32 +++----------
 .../kernel/syscalls/fanotify/fanotify05.c     |  9 +---
 .../kernel/syscalls/fanotify/fanotify06.c     | 21 ++------
 .../kernel/syscalls/fanotify/fanotify07.c     |  9 +---
 .../kernel/syscalls/fanotify/fanotify09.c     | 19 ++------
 .../kernel/syscalls/fanotify/fanotify10.c     | 22 ++-------
 .../kernel/syscalls/fanotify/fanotify11.c     |  5 +-
 .../kernel/syscalls/fanotify/fanotify12.c     | 24 ++--------
 .../kernel/syscalls/fanotify/fanotify13.c     | 18 ++-----
 .../kernel/syscalls/fanotify/fanotify15.c     | 20 ++------
 .../kernel/syscalls/fanotify/fanotify16.c     | 20 ++------
 15 files changed, 77 insertions(+), 225 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 2275a1da3..e0504ba81 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -61,6 +61,29 @@ int safe_fanotify_init(const char *file, const int lineno,
 	return rval;
 }
 
+static inline int safe_fanotify_mark(const char *file, const int lineno,
+			int fd, unsigned int flags, uint64_t mask,
+			int dfd, const char *pathname)
+{
+	int rval;
+
+	rval = fanotify_mark(fd, flags, mask, dfd, pathname);
+
+	if (rval == -1) {
+		tst_brk_(file, lineno, TBROK | TERRNO, "fanotify_mark() failed");
+	}
+
+	if (rval < -1) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			 "invalid fanotify_mark() return %d", rval);
+	}
+
+	return rval;
+}
+
+#define SAFE_FANOTIFY_MARK(fd, flags, mask, dfd, pathname)  \
+	safe_fanotify_mark(__FILE__, __LINE__, (fd), (flags), (mask), (dfd), (pathname))
+
 #define SAFE_FANOTIFY_INIT(fan, mode)  \
 	safe_fanotify_init(__FILE__, __LINE__, (fan), (mode))
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
index c8ab41695..a1eafb277 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify01.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
@@ -102,14 +102,8 @@ static void test_fanotify(unsigned int n)
 
 	fd_notify = SAFE_FANOTIFY_INIT(tc->init_flags, O_RDONLY);
 
-	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
-			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
-			  AT_FDCWD, fname) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS | %s | "
-			"FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, %s) "
-			"failed", fd_notify, mark->name, fname);
-	}
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag,
+			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, fname);
 
 	/*
 	 * generate sequence of events
@@ -157,14 +151,9 @@ static void test_fanotify(unsigned int n)
 	 */
 
 	/* Ignore access events */
-	if (fanotify_mark(fd_notify,
+	SAFE_FANOTIFY_MARK(fd_notify,
 			  FAN_MARK_ADD | mark->flag | FAN_MARK_IGNORED_MASK,
-			  FAN_ACCESS, AT_FDCWD, fname) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD | %s | "
-			"FAN_MARK_IGNORED_MASK, FAN_ACCESS, AT_FDCWD, %s) "
-			"failed", fd_notify, mark->name, fname);
-	}
+			  FAN_ACCESS, AT_FDCWD, fname);
 
 	fd = SAFE_OPEN(fname, O_RDWR);
 	event_set[tst_count] = FAN_OPEN;
@@ -207,15 +196,9 @@ static void test_fanotify(unsigned int n)
 	 * Now ignore open & close events regardless of file
 	 * modifications
 	 */
-	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag |
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag |
 			  FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY,
-			  FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD | %s | "
-			"FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY, "
-			"FAN_OPEN | FAN_CLOSE, AT_FDCWD, %s) failed",
-			fd_notify, mark->name, fname);
-	}
+			  FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname);
 
 	/* This event should be ignored */
 	fd = SAFE_OPEN(fname, O_RDWR);
@@ -236,15 +219,9 @@ static void test_fanotify(unsigned int n)
 	len += ret;
 
 	/* Now remove open and close from ignored mask */
-	if (fanotify_mark(fd_notify,
+	SAFE_FANOTIFY_MARK(fd_notify,
 			  FAN_MARK_REMOVE | mark->flag | FAN_MARK_IGNORED_MASK,
-			  FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_REMOVE | %s | "
-			"FAN_MARK_IGNORED_MASK, FAN_OPEN | FAN_CLOSE, "
-			"AT_FDCWD, %s) failed", fd_notify,
-			mark->name, fname);
-	}
+			  FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname);
 
 	SAFE_CLOSE(fd);
 	event_set[tst_count] = FAN_CLOSE_WRITE;
@@ -340,14 +317,9 @@ pass:
 
 	}
 	/* Remove mark to clear FAN_MARK_IGNORED_SURV_MODIFY */
-	if (fanotify_mark(fd_notify, FAN_MARK_REMOVE | mark->flag,
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE | mark->flag,
 			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
-			  AT_FDCWD, fname) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_REMOVE | %s, FAN_ACCESS | "
-			"FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, %s) "
-			"failed", fd_notify, mark->name, fname);
-	}
+			  AT_FDCWD, fname);
 
 	SAFE_CLOSE(fd_notify);
 }
diff --git a/testcases/kernel/syscalls/fanotify/fanotify02.c b/testcases/kernel/syscalls/fanotify/fanotify02.c
index c578e0ae8..36c87648e 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify02.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify02.c
@@ -46,16 +46,9 @@ void test01(void)
 
 	int tst_count = 0;
 
-	if (fanotify_mark(fd_notify, FAN_MARK_ADD, FAN_ACCESS |
-			  FAN_MODIFY | FAN_CLOSE | FAN_OPEN |
-			  FAN_EVENT_ON_CHILD | FAN_ONDIR, AT_FDCWD,
-			  ".") < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS | "
-			"FAN_MODIFY | FAN_CLOSE | FAN_OPEN | "
-			"FAN_EVENT_ON_CHILD | FAN_ONDIR, AT_FDCWD, '.') "
-			"failed", fd_notify);
-	}
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD, FAN_ACCESS |
+			  FAN_MODIFY | FAN_CLOSE | FAN_OPEN | FAN_EVENT_ON_CHILD |
+			  FAN_ONDIR, AT_FDCWD, ".");
 
 	/*
 	 * generate sequence of events
@@ -102,13 +95,8 @@ void test01(void)
 	/*
 	 * now remove child mark
 	 */
-	if (fanotify_mark(fd_notify, FAN_MARK_REMOVE,
-			  FAN_EVENT_ON_CHILD, AT_FDCWD, ".") < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK REMOVE, "
-			"FAN_EVENT_ON_CHILD, AT_FDCWD, '.') failed",
-			fd_notify);
-	}
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE,
+			  FAN_EVENT_ON_CHILD, AT_FDCWD, ".");
 
 	/*
 	 * Do something to verify events didn't get generated
diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
index 8b54e42c4..2cd90e8b0 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify03.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
@@ -222,14 +222,8 @@ static int setup_mark(unsigned int n)
 	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
 
 	for (; i < ARRAY_SIZE(files); i++) {
-		if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
-				  tc->mask, AT_FDCWD, files[i]) < 0) {
-			tst_brk(TBROK | TERRNO,
-				"fanotify_mark(%d, FAN_MARK_ADD | %s, "
-				"FAN_ACCESS_PERM | FAN_OPEN_PERM, "
-				"AT_FDCWD, %s) failed.",
-				fd_notify, mark->name, fname);
-		}
+		SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag,
+				  tc->mask, AT_FDCWD, files[i]);
 	}
 
 	return 0;
diff --git a/testcases/kernel/syscalls/fanotify/fanotify04.c b/testcases/kernel/syscalls/fanotify/fanotify04.c
index 722ad5d41..a24e7f7c3 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify04.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify04.c
@@ -77,13 +77,8 @@ static void check_mark(char *file, unsigned long long flag, char *flagstr,
 		if (test_event)
 			test_event(file);
 
-		if (fanotify_mark(fd_notify, FAN_MARK_REMOVE | flag,
-				  FAN_OPEN, AT_FDCWD, file) < 0) {
-			tst_brk(TBROK | TERRNO,
-				"fanotify_mark (%d, FAN_MARK_REMOVE | %s, "
-				"FAN_OPEN, AT_FDCWD, '%s') failed",
-				fd_notify, flagstr, file);
-		}
+		SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE | flag,
+				  FAN_OPEN, AT_FDCWD, file);
 	}
 }
 
@@ -191,29 +186,14 @@ void test01(void)
 	CHECK_MARK(sname, 0, 0, test_open_file);
 
 	/* Verify FAN_MARK_FLUSH destroys all inode marks */
-	if (fanotify_mark(fd_notify, FAN_MARK_ADD,
-			  FAN_OPEN, AT_FDCWD, fname) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD, FAN_OPEN, "
-			"AT_FDCWD, '%s') failed", fd_notify, fname);
-	}
-	if (fanotify_mark(fd_notify, FAN_MARK_ADD,
-			  FAN_OPEN | FAN_ONDIR, AT_FDCWD, dir) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD, FAN_OPEN | "
-			"FAN_ONDIR, AT_FDCWD, '%s') failed", fd_notify,
-			dir);
-	}
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD, FAN_OPEN, AT_FDCWD, fname);
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD, FAN_OPEN | FAN_ONDIR,
+			   AT_FDCWD, dir);
 	open_file(fname);
 	verify_event(S_IFREG);
 	open_dir(dir);
 	verify_event(S_IFDIR);
-	if (fanotify_mark(fd_notify, FAN_MARK_FLUSH,
-			  0, AT_FDCWD, ".") < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_FLUSH, 0, "
-			"AT_FDCWD, '.') failed", fd_notify);
-	}
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_FLUSH, 0, AT_FDCWD, ".");
 
 	open_dir(dir);
 	verify_no_event();
diff --git a/testcases/kernel/syscalls/fanotify/fanotify05.c b/testcases/kernel/syscalls/fanotify/fanotify05.c
index e53cc333a..c13b9ad7b 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify05.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify05.c
@@ -107,13 +107,8 @@ static void setup(void)
 	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | FAN_NONBLOCK,
 			O_RDONLY);
 
-	if (fanotify_mark(fd_notify, FAN_MARK_MOUNT | FAN_MARK_ADD, FAN_OPEN,
-			  AT_FDCWD, MOUNT_PATH) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_MOUNT | FAN_MARK_ADD, "
-			"FAN_OPEN, AT_FDCWD, \".\") failed",
-			fd_notify);
-	}
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_MOUNT | FAN_MARK_ADD, FAN_OPEN,
+			  AT_FDCWD, MOUNT_PATH);
 }
 
 static void cleanup(void)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify06.c b/testcases/kernel/syscalls/fanotify/fanotify06.c
index 99e312a4f..ac4901f6f 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify06.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify06.c
@@ -78,7 +78,6 @@ static struct tcase {
 static void create_fanotify_groups(void)
 {
 	unsigned int p, i;
-	int ret;
 
 	for (p = 0; p < FANOTIFY_PRIORITIES; p++) {
 		for (i = 0; i < GROUPS_PER_PRIO; i++) {
@@ -87,32 +86,20 @@ static void create_fanotify_groups(void)
 							     O_RDONLY);
 
 			/* Add mount mark for each group */
-			ret = fanotify_mark(fd_notify[p][i],
+			SAFE_FANOTIFY_MARK(fd_notify[p][i],
 					    FAN_MARK_ADD | FAN_MARK_MOUNT,
 					    FAN_MODIFY,
 					    AT_FDCWD, fname);
-			if (ret < 0) {
-				tst_brk(TBROK | TERRNO,
-					"fanotify_mark(%d, FAN_MARK_ADD | "
-					"FAN_MARK_MOUNT, FAN_MODIFY, AT_FDCWD,"
-					" %s) failed", fd_notify[p][i], fname);
-			}
+
 			/* Add ignore mark for groups with higher priority */
 			if (p == 0)
 				continue;
-			ret = fanotify_mark(fd_notify[p][i],
+
+			SAFE_FANOTIFY_MARK(fd_notify[p][i],
 					    FAN_MARK_ADD |
 					    FAN_MARK_IGNORED_MASK |
 					    FAN_MARK_IGNORED_SURV_MODIFY,
 					    FAN_MODIFY, AT_FDCWD, fname);
-			if (ret < 0) {
-				tst_brk(TBROK | TERRNO,
-					"fanotify_mark(%d, FAN_MARK_ADD | "
-					"FAN_MARK_IGNORED_MASK | "
-					"FAN_MARK_IGNORED_SURV_MODIFY, "
-					"FAN_MODIFY, AT_FDCWD, %s) failed",
-					fd_notify[p][i], fname);
-			}
 		}
 	}
 }
diff --git a/testcases/kernel/syscalls/fanotify/fanotify07.c b/testcases/kernel/syscalls/fanotify/fanotify07.c
index 8822e9c51..4bf17661a 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify07.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify07.c
@@ -102,14 +102,7 @@ static int setup_instance(void)
 	int fd;
 
 	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
-
-	if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD,
-			  fname) < 0) {
-		close(fd);
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, %s) failed",
-			fd, fname);
-	}
+	SAFE_FANOTIFY_MARK(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, fname);
 
 	return fd;
 }
diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
index 83210bc1c..9c9938619 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify09.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
@@ -99,7 +99,6 @@ static void create_fanotify_groups(struct tcase *tc)
 {
 	struct fanotify_mark_type *mark = &tc->mark;
 	unsigned int i, onchild, ondir = tc->ondir;
-	int ret;
 
 	for (i = 0; i < NUM_GROUPS; i++) {
 		fd_notify[i] = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF |
@@ -111,17 +110,11 @@ static void create_fanotify_groups(struct tcase *tc)
 		 * but only the first group requests events on dir.
 		 */
 		onchild = (i == 0) ? FAN_EVENT_ON_CHILD | ondir : 0;
-		ret = fanotify_mark(fd_notify[i],
+		SAFE_FANOTIFY_MARK(fd_notify[i],
 				    FAN_MARK_ADD | mark->flag,
 				    FAN_CLOSE_NOWRITE | onchild,
 				    AT_FDCWD, tc->testdir);
-		if (ret < 0) {
-			tst_brk(TBROK | TERRNO,
-				"fanotify_mark(%d, FAN_MARK_ADD | %s, "
-				"%x, AT_FDCWD, '%s') failed",
-				fd_notify[i], mark->name,
-				FAN_CLOSE_NOWRITE | ondir, tc->testdir);
-		}
+
 		/*
 		 * Add inode mark on parent for each group with MODIFY event,
 		 * but only the first group requests events on child.
@@ -129,15 +122,9 @@ static void create_fanotify_groups(struct tcase *tc)
 		 * setting the DCACHE_FSNOTIFY_PARENT_WATCHED dentry
 		 * flag.
 		 */
-		ret = fanotify_mark(fd_notify[i], FAN_MARK_ADD,
+		SAFE_FANOTIFY_MARK(fd_notify[i], FAN_MARK_ADD,
 				    FAN_MODIFY | ondir | onchild,
 				    AT_FDCWD, ".");
-		if (ret < 0) {
-			tst_brk(TBROK | TERRNO,
-				"fanotify_mark(%d, FAN_MARK_ADD, "
-				"%x, AT_FDCWD, '.') failed",
-				fd_notify[i], FAN_MODIFY | ondir | onchild);
-		}
 	}
 }
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index 6d048958c..4d081a843 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -289,7 +289,6 @@ static int create_fanotify_groups(unsigned int n)
 	struct fanotify_mark_type *mark, *ignore_mark;
 	unsigned int mark_ignored, mask;
 	unsigned int p, i;
-	int ret;
 
 	mark = &fanotify_mark_types[tc->mark_type];
 	ignore_mark = &fanotify_mark_types[tc->ignore_mark_type];
@@ -318,18 +317,12 @@ static int create_fanotify_groups(unsigned int n)
 			 * FAN_EVENT_ON_CHILD has no effect on filesystem/mount
 			 * or inode mark on non-directory.
 			 */
-			ret = fanotify_mark(fd_notify[p][i],
+			SAFE_FANOTIFY_MARK(fd_notify[p][i],
 					    FAN_MARK_ADD | mark->flag,
 					    tc->expected_mask_without_ignore |
 					    FAN_EVENT_ON_CHILD,
 					    AT_FDCWD, tc->mark_path);
-			if (ret < 0) {
-				tst_brk(TBROK | TERRNO,
-					"fanotify_mark(%d, FAN_MARK_ADD | %s,"
-					"FAN_OPEN, AT_FDCWD, %s) failed",
-					fd_notify[p][i], mark->name,
-					tc->mark_path);
-			}
+
 			/* Add ignore mark for groups with higher priority */
 			if (p == 0)
 				continue;
@@ -338,18 +331,9 @@ static int create_fanotify_groups(unsigned int n)
 			mark_ignored = FAN_MARK_IGNORED_MASK |
 					FAN_MARK_IGNORED_SURV_MODIFY;
 add_mark:
-			ret = fanotify_mark(fd_notify[p][i],
+			SAFE_FANOTIFY_MARK(fd_notify[p][i],
 					    FAN_MARK_ADD | ignore_mark->flag | mark_ignored,
 					    mask, AT_FDCWD, tc->ignore_path);
-			if (ret < 0) {
-				tst_brk(TBROK | TERRNO,
-					"fanotify_mark(%d, FAN_MARK_ADD | %s | %s, "
-					"%x, AT_FDCWD, %s) failed",
-					fd_notify[p][i], ignore_mark->name,
-					mark_ignored ? "FAN_MARK_IGNORED_MASK | "
-					"FAN_MARK_IGNORED_SURV_MODIFY" : "",
-					mask, tc->ignore_path);
-			}
 
 			/*
 			 * If ignored mask is on a parent watching children,
diff --git a/testcases/kernel/syscalls/fanotify/fanotify11.c b/testcases/kernel/syscalls/fanotify/fanotify11.c
index 9e8606c72..56b7153f8 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify11.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify11.c
@@ -55,7 +55,6 @@ static unsigned int tcases[] = {
 
 void test01(unsigned int i)
 {
-	int ret;
 	pthread_t p_id;
 	struct fanotify_event_metadata event;
 	int fd_notify;
@@ -76,10 +75,8 @@ void test01(unsigned int i)
 				tcases[i]);
 	}
 
-	ret = fanotify_mark(fd_notify, FAN_MARK_ADD,
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD,
 			FAN_ALL_EVENTS | FAN_EVENT_ON_CHILD, AT_FDCWD, ".");
-	if (ret != 0)
-		tst_brk(TBROK, "fanotify_mark FAN_MARK_ADD fail ret=%d", ret);
 
 	SAFE_PTHREAD_CREATE(&p_id, NULL, thread_create_file, NULL);
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
index 0d049fc6b..18b96c430 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify12.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
@@ -147,31 +147,15 @@ static int setup_mark(unsigned int n)
 
 	for (; i < ARRAY_SIZE(files); i++) {
 		/* Setup normal mark on object */
-		if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
-					tc->mask, AT_FDCWD, files[i]) < 0) {
-			tst_brk(TBROK | TERRNO,
-				"fanotify_mark(%d, FAN_MARK_ADD | %s, "
-				"%llx, AT_FDCWD, %s) failed",
-				fd_notify,
-				mark->name,
-				tc->mask,
-				files[i]);
-		}
+		SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag,
+					tc->mask, AT_FDCWD, files[i]);
 
 		/* Setup ignore mark on object */
 		if (tc->ignore_mask) {
-			if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag
+			SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag
 						| FAN_MARK_IGNORED_MASK,
 						tc->ignore_mask, AT_FDCWD,
-						files[i]) < 0) {
-				tst_brk(TBROK | TERRNO,
-					"fanotify_mark (%d, "
-					"FAN_MARK_ADD | %s "
-					"| FAN_MARK_IGNORED_MASK, "
-					"%llx, AT_FDCWD, %s) failed",
-					fd_notify, mark->name,
-					tc->ignore_mask, files[i]);
-			}
+						files[i]);
 		}
 	}
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index 33989d902..d28d1a6de 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -117,13 +117,8 @@ static int setup_marks(unsigned int fd, struct test_case_t *tc)
 	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) {
-			tst_brk(TBROK | TERRNO,
-				"fanotify_mark(%d, FAN_MARK_ADD, FAN_OPEN, "
-				"AT_FDCWD, %s) failed",
-				fanotify_fd, objects[i].path);
-		}
+		SAFE_FANOTIFY_MARK(fd, FAN_MARK_ADD | mark->flag, tc->mask,
+				   AT_FDCWD, objects[i].path);
 
 		/* Setup the expected mask for each generated event */
 		event_set[i].expected_mask = tc->mask;
@@ -276,13 +271,8 @@ static void do_setup(void)
 	 * uninitialized connector->fsid cache. This mark remains for all test
 	 * cases and is not expected to get any events (no writes in this test).
 	 */
-	if (fanotify_mark(nofid_fd, FAN_MARK_ADD, FAN_CLOSE_WRITE, AT_FDCWD,
-			  FILE_PATH_ONE) == -1) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark(%d, FAN_MARK_ADD, FAN_CLOSE_WRITE, "
-			"AT_FDCWD, "FILE_PATH_ONE") failed",
-			nofid_fd);
-	}
+	SAFE_FANOTIFY_MARK(nofid_fd, FAN_MARK_ADD, FAN_CLOSE_WRITE, AT_FDCWD,
+			  FILE_PATH_ONE);
 
 	/* Get the filesystem fsid and file handle for each created object */
 	get_object_stats();
diff --git a/testcases/kernel/syscalls/fanotify/fanotify15.c b/testcases/kernel/syscalls/fanotify/fanotify15.c
index 2e3044ad7..6dd5de699 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify15.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify15.c
@@ -85,17 +85,10 @@ static void do_test(unsigned int number)
 
 	tst_res(TINFO, "Test #%d: %s", number, tc->tname);
 
-	if (fanotify_mark(fanotify_fd, FAN_MARK_ADD | mark->flag, tc->mask |
+	SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD | mark->flag, tc->mask |
 				FAN_CREATE | FAN_DELETE | FAN_MOVE |
 				FAN_MODIFY | FAN_ONDIR,
-				AT_FDCWD, TEST_DIR) == -1) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark(%d, FAN_MARK_ADD | %s, "
-			"FAN_CREATE | FAN_DELETE | FAN_MOVE | "
-			"FAN_MODIFY | FAN_ONDIR | 0x%lx, "
-			"AT_FDCWD, %s) failed",
-			fanotify_fd, mark->name, tc->mask, TEST_DIR);
-	}
+				AT_FDCWD, TEST_DIR);
 
 	/* Save the test root dir fid */
 	fanotify_save_fid(TEST_DIR, &root_fid);
@@ -187,13 +180,8 @@ static void do_test(unsigned int number)
 	/*
 	 * Cleanup the mark
 	 */
-	if (fanotify_mark(fanotify_fd, FAN_MARK_FLUSH | mark->flag, 0,
-			  AT_FDCWD, TEST_DIR) < 0) {
-		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_FLUSH | %s, 0,"
-			"AT_FDCWD, '"TEST_DIR"') failed",
-			fanotify_fd, mark->name);
-	}
+	SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_FLUSH | mark->flag, 0,
+			  AT_FDCWD, TEST_DIR);
 
 	/* Process each event in buffer */
 	for (i = 0, metadata = (struct fanotify_event_metadata *) events_buf;
diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
index a379226f8..0e4afac13 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify16.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify16.c
@@ -174,13 +174,8 @@ static void do_test(unsigned int number)
 	/*
 	 * Watch dir modify events with name in filesystem/dir
 	 */
-	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
-			  AT_FDCWD, MOUNT_PATH) < 0) {
-		tst_brk(TBROK | TERRNO,
-		    "fanotify_mark (%d, FAN_MARK_ADD | %s, 0x%lx, "
-		    "AT_FDCWD, '"MOUNT_PATH"') failed",
-		    fd_notify, mark->name, tc->mask);
-	}
+	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
+			   AT_FDCWD, MOUNT_PATH);
 
 	/* Save the mount root fid */
 	fanotify_save_fid(MOUNT_PATH, &root_fid);
@@ -195,14 +190,9 @@ static void do_test(unsigned int number)
 	/* Save the subdir fid */
 	fanotify_save_fid(dname1, &dir_fid);
 
-	if (tc->sub_mask &&
-	    fanotify_mark(fd_notify, FAN_MARK_ADD | sub_mark->flag, tc->sub_mask,
-			  AT_FDCWD, dname1) < 0) {
-		tst_brk(TBROK | TERRNO,
-		    "fanotify_mark (%d, FAN_MARK_ADD | %s, 0x%lx, "
-		    "AT_FDCWD, '%s') failed",
-		    fd_notify, sub_mark->name, tc->sub_mask, dname1);
-	}
+	if (tc->sub_mask)
+		SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | sub_mark->flag,
+				   tc->sub_mask, AT_FDCWD, dname1);
 
 	event_set[tst_count].mask = FAN_CREATE | FAN_ONDIR;
 	event_set[tst_count].fid = &root_fid;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 08/10] fanotify: Use tst_brk_ in safe_fanotify_init()
  2020-12-01 17:42 [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
                   ` (6 preceding siblings ...)
  2020-12-01 17:42 ` [LTP] [PATCH v5 07/10] fanotify: Introduce SAFE_FANOTIFY_MARK() macro Petr Vorel
@ 2020-12-01 17:42 ` Petr Vorel
  2020-12-02 15:55   ` Cyril Hrubis
  2020-12-01 17:42 ` [LTP] [PATCH v5 09/10] fanotify: Add a pedantic check for return value Petr Vorel
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Petr Vorel @ 2020-12-01 17:42 UTC (permalink / raw)
  To: ltp

This show correct file and line number
(sync with safe_fanotify_mark()).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v5 (previously part of "[v4,5/6] fanotify: Check FAN_REPORT_{FID,
NAME} support").

 testcases/kernel/syscalls/fanotify/fanotify.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index e0504ba81..0e99b4ac1 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -48,14 +48,14 @@ int safe_fanotify_init(const char *file, const int lineno,
 
 	if (rval == -1) {
 		if (errno == ENOSYS) {
-			tst_brk(TCONF,
-				"fanotify is not configured in this kernel.");
+			tst_brk_(file, lineno, TCONF,
+				"fanotify is not configured in this kernel");
 		}
-		tst_brk(TBROK | TERRNO,
+		tst_brk_(file, lineno, TBROK | TERRNO,
 			"%s:%d: fanotify_init() failed", file, lineno);
 	}
 #else
-	tst_brk(TCONF, "Header <sys/fanotify.h> is not present");
+	tst_brk_(file, lineno, TCONF, "Header <sys/fanotify.h> is not present");
 #endif /* HAVE_SYS_FANOTIFY_H */
 
 	return rval;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 09/10] fanotify: Add a pedantic check for return value
  2020-12-01 17:42 [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
                   ` (7 preceding siblings ...)
  2020-12-01 17:42 ` [LTP] [PATCH v5 08/10] fanotify: Use tst_brk_ in safe_fanotify_init() Petr Vorel
@ 2020-12-01 17:42 ` Petr Vorel
  2020-12-02 15:56   ` Cyril Hrubis
  2020-12-01 17:42 ` [LTP] [PATCH] fanotify: Cleanup <sys/fanotify.h> use Petr Vorel
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Petr Vorel @ 2020-12-01 17:42 UTC (permalink / raw)
  To: ltp

for fanotify_init() in safe_fanotify_init()

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Same as v4.

 testcases/kernel/syscalls/fanotify/fanotify.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 0e99b4ac1..540c2b0fc 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -54,6 +54,11 @@ int safe_fanotify_init(const char *file, const int lineno,
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"%s:%d: fanotify_init() failed", file, lineno);
 	}
+
+	if (rval < -1) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			 "invalid fanotify_init() return %d", rval);
+	}
 #else
 	tst_brk_(file, lineno, TCONF, "Header <sys/fanotify.h> is not present");
 #endif /* HAVE_SYS_FANOTIFY_H */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [LTP] [PATCH] fanotify: Cleanup <sys/fanotify.h> use
  2020-12-01 17:42 [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
                   ` (8 preceding siblings ...)
  2020-12-01 17:42 ` [LTP] [PATCH v5 09/10] fanotify: Add a pedantic check for return value Petr Vorel
@ 2020-12-01 17:42 ` Petr Vorel
  2020-12-02 16:12   ` Cyril Hrubis
  2020-12-01 17:46 ` [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
  2020-12-02  6:47 ` Amir Goldstein
  11 siblings, 1 reply; 30+ messages in thread
From: Petr Vorel @ 2020-12-01 17:42 UTC (permalink / raw)
  To: ltp

fanotify.h is included only by fanotify tests. That's why fb2cd7934
moved safe_fanotify_{init,mark}() was moved into this header.
All fanotify tests require <sys/fanotify.h> anyway, thus move inclusion
behind HAVE_SYS_FANOTIFY_H guard and remove checks and raw syscall
fallbacks from the header.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v5.

It should be pretty obvious. There are probably some unused includes,
but again, let's don't prolong this patchset.

 testcases/kernel/syscalls/fanotify/fanotify.h | 27 -------------------
 .../kernel/syscalls/fanotify/fanotify01.c     |  5 ++--
 .../kernel/syscalls/fanotify/fanotify02.c     |  5 ++--
 .../kernel/syscalls/fanotify/fanotify03.c     |  5 ++--
 .../kernel/syscalls/fanotify/fanotify04.c     |  5 ++--
 .../kernel/syscalls/fanotify/fanotify05.c     |  5 ++--
 .../kernel/syscalls/fanotify/fanotify06.c     |  5 ++--
 .../kernel/syscalls/fanotify/fanotify07.c     |  5 ++--
 .../kernel/syscalls/fanotify/fanotify08.c     |  5 ++--
 .../kernel/syscalls/fanotify/fanotify09.c     |  5 ++--
 .../kernel/syscalls/fanotify/fanotify10.c     |  5 ++--
 .../kernel/syscalls/fanotify/fanotify11.c     |  7 +++--
 .../kernel/syscalls/fanotify/fanotify12.c     |  5 ++--
 .../kernel/syscalls/fanotify/fanotify13.c     |  5 ++--
 .../kernel/syscalls/fanotify/fanotify14.c     |  6 ++---
 .../kernel/syscalls/fanotify/fanotify15.c     |  6 ++---
 .../kernel/syscalls/fanotify/fanotify16.c     |  5 ++--
 17 files changed, 33 insertions(+), 78 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 540c2b0fc..ea3771cce 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -13,37 +13,13 @@
 #include <sys/stat.h>
 #include <errno.h>
 #include <fcntl.h>
-
-#if defined(HAVE_SYS_FANOTIFY_H)
-
 #include <sys/fanotify.h>
 
-#else /* HAVE_SYS_FANOTIFY_H */
-
-/* fanotify(7) wrappers */
-
-#include <stdint.h>
-#include "lapi/syscalls.h"
-
-static int fanotify_init(unsigned int flags, unsigned int event_f_flags)
-{
-	return syscall(__NR_fanotify_init, flags, event_f_flags);
-}
-
-static long fanotify_mark(int fd, unsigned int flags, uint64_t mask,
-                     int dfd, const char *pathname)
-{
-	return syscall(__NR_fanotify_mark, fd, flags, mask, dfd, pathname);
-}
-
-#endif /* HAVE_SYS_FANOTIFY_H */
-
 int safe_fanotify_init(const char *file, const int lineno,
 	unsigned int flags, unsigned int event_f_flags)
 {
 	int rval;
 
-#ifdef HAVE_SYS_FANOTIFY_H
 	rval = fanotify_init(flags, event_f_flags);
 
 	if (rval == -1) {
@@ -59,9 +35,6 @@ int safe_fanotify_init(const char *file, const int lineno,
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			 "invalid fanotify_init() return %d", rval);
 	}
-#else
-	tst_brk_(file, lineno, TCONF, "Header <sys/fanotify.h> is not present");
-#endif /* HAVE_SYS_FANOTIFY_H */
 
 	return rval;
 }
diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
index a1eafb277..d6d72dad9 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify01.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
@@ -18,10 +18,9 @@
 #include <string.h>
 #include <sys/syscall.h>
 #include "tst_test.h"
-#include "fanotify.h"
 
-#if defined(HAVE_SYS_FANOTIFY_H)
-#include <sys/fanotify.h>
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
 
 #define EVENT_MAX 1024
 /* size of the event structure, not counting name */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify02.c b/testcases/kernel/syscalls/fanotify/fanotify02.c
index 36c87648e..155875dd0 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify02.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify02.c
@@ -18,10 +18,9 @@
 #include <string.h>
 #include <sys/syscall.h>
 #include "tst_test.h"
-#include "fanotify.h"
 
-#if defined(HAVE_SYS_FANOTIFY_H)
-#include <sys/fanotify.h>
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
 
 #define EVENT_MAX 1024
 /* size of the event structure, not counting name */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
index 2cd90e8b0..1368bf0e2 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify03.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
@@ -22,10 +22,9 @@
 #include <sys/syscall.h>
 #include <stdlib.h>
 #include "tst_test.h"
-#include "fanotify.h"
 
-#if defined(HAVE_SYS_FANOTIFY_H)
-#include <sys/fanotify.h>
+#ifdef HAVE_SYS_FANOTIFY_H
+# include "fanotify.h"
 
 #define EVENT_MAX 1024
 /* size of the event structure, not counting name */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify04.c b/testcases/kernel/syscalls/fanotify/fanotify04.c
index a24e7f7c3..7ac2645bb 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify04.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify04.c
@@ -19,10 +19,9 @@
 #include <string.h>
 #include <sys/syscall.h>
 #include "tst_test.h"
-#include "fanotify.h"
 
-#if defined(HAVE_SYS_FANOTIFY_H)
-#include <sys/fanotify.h>
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
 
 #define EVENT_MAX 1024
 /* size of the event structure, not counting name */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify05.c b/testcases/kernel/syscalls/fanotify/fanotify05.c
index c13b9ad7b..851cb6c8f 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify05.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify05.c
@@ -22,10 +22,9 @@
 #include <string.h>
 #include <sys/syscall.h>
 #include "tst_test.h"
-#include "fanotify.h"
 
-#if defined(HAVE_SYS_FANOTIFY_H)
-#include <sys/fanotify.h>
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
 
 #define MOUNT_PATH "fs_mnt"
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify06.c b/testcases/kernel/syscalls/fanotify/fanotify06.c
index ac4901f6f..0d70a955f 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify06.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify06.c
@@ -36,10 +36,9 @@
 #include <sys/mount.h>
 #include <sys/syscall.h>
 #include "tst_test.h"
-#include "fanotify.h"
 
-#if defined(HAVE_SYS_FANOTIFY_H)
-#include <sys/fanotify.h>
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
 
 #define EVENT_MAX 1024
 /* size of the event structure, not counting name */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify07.c b/testcases/kernel/syscalls/fanotify/fanotify07.c
index 4bf17661a..830af32b6 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify07.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify07.c
@@ -30,10 +30,9 @@
 #include <sys/syscall.h>
 #include "tst_test.h"
 #include "lapi/syscalls.h"
-#include "fanotify.h"
 
-#if defined(HAVE_SYS_FANOTIFY_H)
-#include <sys/fanotify.h>
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
 
 #define BUF_SIZE 256
 static char fname[BUF_SIZE];
diff --git a/testcases/kernel/syscalls/fanotify/fanotify08.c b/testcases/kernel/syscalls/fanotify/fanotify08.c
index a4031b4ad..f63b5a931 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify08.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify08.c
@@ -18,10 +18,9 @@
 #include <string.h>
 #include <sys/syscall.h>
 #include "tst_test.h"
-#include "fanotify.h"
 
-#if defined(HAVE_SYS_FANOTIFY_H)
-#include <sys/fanotify.h>
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
 
 static int fd_notify;
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
index 9c9938619..daeb712d2 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify09.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
@@ -33,10 +33,9 @@
 #include <sys/syscall.h>
 #include <stdint.h>
 #include "tst_test.h"
-#include "fanotify.h"
 
-#if defined(HAVE_SYS_FANOTIFY_H)
-#include <sys/fanotify.h>
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
 
 #define EVENT_MAX 1024
 /* size of the event structure, not counting name */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index 4d081a843..404e57daa 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -39,10 +39,9 @@
 #include <sys/mount.h>
 #include <sys/syscall.h>
 #include "tst_test.h"
-#include "fanotify.h"
 
-#if defined(HAVE_SYS_FANOTIFY_H)
-#include <sys/fanotify.h>
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
 
 #define EVENT_MAX 1024
 /* size of the event structure, not counting name */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify11.c b/testcases/kernel/syscalls/fanotify/fanotify11.c
index 56b7153f8..785b5c5a5 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify11.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify11.c
@@ -29,10 +29,9 @@
 #include <linux/limits.h>
 #include "tst_test.h"
 #include "tst_safe_pthread.h"
-#include "fanotify.h"
 
-#if defined(HAVE_SYS_FANOTIFY_H)
-#include <sys/fanotify.h>
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
 
 #define gettid() syscall(SYS_gettid)
 static int tid;
@@ -112,5 +111,5 @@ static struct tst_test test = {
 };
 
 #else
-TST_TEST_TCONF("system doesn't have required fanotify support");
+	TST_TEST_TCONF("system doesn't have required fanotify support");
 #endif
diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
index 18b96c430..17086ef71 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify12.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
@@ -21,10 +21,9 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 #include "tst_test.h"
-#include "fanotify.h"
 
-#if defined(HAVE_SYS_FANOTIFY_H)
-#include <sys/fanotify.h>
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
 
 #define EVENT_MAX 1024
 #define EVENT_SIZE (sizeof (struct fanotify_event_metadata))
diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index d28d1a6de..a402cdb13 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -25,10 +25,9 @@
 #include <errno.h>
 #include <unistd.h>
 #include "tst_test.h"
-#include "fanotify.h"
 
-#if defined(HAVE_SYS_FANOTIFY_H)
-#include <sys/fanotify.h>
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
 
 #define PATH_LEN 128
 #define BUF_SIZE 256
diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
index 349177d9a..817d5ecd5 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify14.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
@@ -12,12 +12,10 @@
  */
 #define _GNU_SOURCE
 #include "tst_test.h"
-#include "fanotify.h"
-
 #include <errno.h>
 
-#if defined(HAVE_SYS_FANOTIFY_H)
-#include <sys/fanotify.h>
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
 
 #define MNTPOINT "mntpoint"
 #define FILE1 MNTPOINT"/file1"
diff --git a/testcases/kernel/syscalls/fanotify/fanotify15.c b/testcases/kernel/syscalls/fanotify/fanotify15.c
index 6dd5de699..9e3748bc2 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify15.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify15.c
@@ -22,12 +22,10 @@
 #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>
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
 
 #define EVENT_MAX 10
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
index 0e4afac13..a554c7d39 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify16.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify16.c
@@ -24,10 +24,9 @@
 #include <sys/mount.h>
 #include <sys/syscall.h>
 #include "tst_test.h"
-#include "fanotify.h"
 
-#if defined(HAVE_SYS_FANOTIFY_H)
-#include <sys/fanotify.h>
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
 
 #define EVENT_MAX 20
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup
  2020-12-01 17:42 [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
                   ` (9 preceding siblings ...)
  2020-12-01 17:42 ` [LTP] [PATCH] fanotify: Cleanup <sys/fanotify.h> use Petr Vorel
@ 2020-12-01 17:46 ` Petr Vorel
  2020-12-02  6:47 ` Amir Goldstein
  11 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-01 17:46 UTC (permalink / raw)
  To: ltp

Hi Amir,

> Changes v4->v5:
...
> Petr Vorel (10):
>   fanotify12: Drop incorrect hint
>   fanotify: Add helper for access permission check
>   fanotify: Add helper for event support check
>   fanotify: Add helper for FAN_REPORT_FID support on fs
>   fanotify16: Test also on FUSE
>   fanotify: Add helper for mark support check
>   fanotify: Introduce SAFE_FANOTIFY_MARK() macro
>   fanotify: Use tst_brk_ in safe_fanotify_init()
>   fanotify: Add a pedantic check for return value

>   fanotify: Cleanup <sys/fanotify.h> use
I omit v5 from this patchset [1], it should have been part of v5.

Kind regards,
Petr

[1] https://patchwork.ozlabs.org/project/ltp/patch/20201201174214.24625-11-pvorel@suse.cz/
[2] https://patchwork.ozlabs.org/project/ltp/list/?series=217855&state=*

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 06/10] fanotify: Add helper for mark support check
  2020-12-01 17:42 ` [LTP] [PATCH v5 06/10] fanotify: Add helper for mark support check Petr Vorel
@ 2020-12-02  6:27   ` Amir Goldstein
  2020-12-02 15:57     ` Cyril Hrubis
  2020-12-02 17:05     ` Petr Vorel
  2020-12-02 15:53   ` Cyril Hrubis
  1 sibling, 2 replies; 30+ messages in thread
From: Amir Goldstein @ 2020-12-02  6:27 UTC (permalink / raw)
  To: ltp

On Tue, Dec 1, 2020 at 7:42 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> i.e. FAN_MARK_FILESYSTEM and use it in relevant tests setup().
> The purpose is to reduce checks (cleanup).
>
> NOTE: all tests check only for FAN_MARK_FILESYSTEM, but keep helper
> generic for future use.
>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> New in v5.
>
>  testcases/kernel/syscalls/fanotify/fanotify.h | 21 ++++++++++++++++
>  .../kernel/syscalls/fanotify/fanotify01.c     | 12 +++++----
>  .../kernel/syscalls/fanotify/fanotify03.c     | 25 +++++++++----------
>  .../kernel/syscalls/fanotify/fanotify10.c     | 14 +++++------
>  .../kernel/syscalls/fanotify/fanotify13.c     | 15 +++++------
>  5 files changed, 55 insertions(+), 32 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index 821e6cb29..2275a1da3 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -351,4 +351,25 @@ static inline void require_fanotify_fan_report_fid_supported_on_fs(const char *f
>  #define REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(fname) \
>         require_fanotify_fan_report_fid_supported_on_fs(__FILE__, __LINE__, (fname))
>
> +static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
> +{
> +       int fd;
> +       int rval = 0;
> +
> +       fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
> +
> +       if (fanotify_mark(fd, FAN_MARK_ADD | flag, FAN_ACCESS_PERM, AT_FDCWD, ".") < 0) {
> +               if (errno == EINVAL) {
> +                       rval = -1;
> +               } else {
> +                       tst_brk(TBROK | TERRNO,
> +                               "fanotify_mark (%d, FAN_MARK_ADD, ..., FAN_ACCESS_PERM, AT_FDCWD, \".\") failed", fd);
> +               }

You meant FAN_ACCESS.

FAN_ACCESS_PERM requires CONFIG_FANOTIFY_ACCESS_PERMISSIONS which is not
a requirement for most of the affected tests.

Patch set is super! it's the only issue I found, so I don't think you
need to re-post for that.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup
  2020-12-01 17:42 [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
                   ` (10 preceding siblings ...)
  2020-12-01 17:46 ` [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
@ 2020-12-02  6:47 ` Amir Goldstein
  11 siblings, 0 replies; 30+ messages in thread
From: Amir Goldstein @ 2020-12-02  6:47 UTC (permalink / raw)
  To: ltp

On Tue, Dec 1, 2020 at 7:42 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Amir,
>
> Changes v4->v5:
> * Fix issue with "fanotify: Add helper for FAN_REPORT_FID support on fs"
>   on fanotify01 (unwanted skipping tests). FAN_REPORT_FID is now tested
>   both for general support in kernel with fanotify_init() and support on
>   tested filesystem in fanotify_mark().
>
> * Fix issue with FAN_MARK_FILESYSTEM (new commit "fanotify: Add helper
>   for mark support check").
>
> * Split "[v4,2/6] fanotify: Handle supported features checks in setup()"
>   into two commits:
>   fanotify: Add helper for access permission check
>   fanotify: Add helper for event support check
>   (easier to review).
>
> * Drop commit "[v4,5/6] fanotify: Check FAN_REPORT_{FID, NAME} support"
>   => IMHO not needed now, as there are {REQUIRE_,}FANOTIFY_FAN_REPORT_FID_ERR_MSG()
>   helpers which check for FAN_REPORT_FID. Or am I'm wrong and you need it
>   for your patchset?

You are right.
My patch set can and will use FANOTIFY_FAN_REPORT_FID_ERR_MSG.

>   There will be needed to add also helper for FAN_REPORT_NAME/FAN_REPORT_DIR_FID
>   for fanotify10.c (for kernels > 5.1 && < 5.9), but this is now covered by check
>   in create_fanotify_groups() and I don't want to block your patchset even more.
>   And this helper should eliminate a need for check in safe_fanotify_init().
>

My patch set grows another test case with FAN_REPORT_NAME to fanotify09
so it is more than fare to leave that last cleanup to me ;-)

Looking forward for the merge of your work.
Thanks!

Amir.

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 01/10] fanotify12: Drop incorrect hint
  2020-12-01 17:42 ` [LTP] [PATCH v5 01/10] fanotify12: Drop incorrect hint Petr Vorel
@ 2020-12-02 15:09   ` Cyril Hrubis
  0 siblings, 0 replies; 30+ messages in thread
From: Cyril Hrubis @ 2020-12-02 15:09 UTC (permalink / raw)
  To: ltp

Hi!

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 02/10] fanotify: Add helper for access permission check
  2020-12-01 17:42 ` [LTP] [PATCH v5 02/10] fanotify: Add helper for access permission check Petr Vorel
@ 2020-12-02 15:24   ` Cyril Hrubis
  0 siblings, 0 replies; 30+ messages in thread
From: Cyril Hrubis @ 2020-12-02 15:24 UTC (permalink / raw)
  To: ltp

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 03/10] fanotify: Add helper for event support check
  2020-12-01 17:42 ` [LTP] [PATCH v5 03/10] fanotify: Add helper for event support check Petr Vorel
@ 2020-12-02 15:36   ` Cyril Hrubis
  0 siblings, 0 replies; 30+ messages in thread
From: Cyril Hrubis @ 2020-12-02 15:36 UTC (permalink / raw)
  To: ltp

Hi!

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 04/10] fanotify: Add helper for FAN_REPORT_FID support on fs
  2020-12-01 17:42 ` [LTP] [PATCH v5 04/10] fanotify: Add helper for FAN_REPORT_FID support on fs Petr Vorel
@ 2020-12-02 15:46   ` Cyril Hrubis
  2020-12-02 18:04     ` Petr Vorel
  0 siblings, 1 reply; 30+ messages in thread
From: Cyril Hrubis @ 2020-12-02 15:46 UTC (permalink / raw)
  To: ltp

Hi!
> +/*
> + * @return  0: fanotify supported both in kernel and on tested filesystem
> + * @return -1: FAN_REPORT_FID not supported in kernel
> + * @return -2: FAN_REPORT_FID not supported on tested filesystem
> + */
> +static inline int fanotify_fan_report_fid_supported_on_fs(const char *fname)
> +{
> +	int fd;
> +	int rval = 0;
> +
> +	fd = fanotify_init(FAN_CLASS_NOTIF | FAN_REPORT_FID, O_RDONLY);
> +
> +	if (fd < 0) {
> +		if (errno == ENOSYS)
> +			tst_brk(TCONF, "fanotify not configured in kernel");
> +
> +		if (errno == EINVAL)
> +			return -1;
> +
> +		tst_brk(TBROK | TERRNO, "fanotify_init() failed");
> +	}
> +
> +	if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS, AT_FDCWD, fname) < 0) {
> +		if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV) {
> +			rval = -2;
> +		} else {
> +			tst_brk(TBROK | TERRNO,
> +				"fanotify_mark (%d, FAN_MARK_ADD, ..., AT_FDCWD, \".\") failed", fd);
                                                                                   ^
										   fname?
> +		}
> +	}
> +
> +	SAFE_CLOSE(fd);
> +
> +	return rval;
> +}
> +
> +typedef void (*tst_res_func_t)(const char *file, const int lineno,
> +		int ttype, const char *fmt, ...);
> +
> +static inline void fanotify_fan_report_fid_err_msg(const char *file,
> +	const int lineno, tst_res_func_t res_func, int fail)
> +{
> +	if (fail == -1)
> +		res_func(file, lineno, TCONF,
> +			 "FAN_REPORT_FID not supported in kernel?");
> +
> +	if (fail == -2)
> +		res_func(file, lineno, TCONF,
> +			 "FAN_REPORT_FID not supported on %s filesystem",
> +			 tst_device->fs_type);
> +}

Maybe this would be just easier to read as a macro:

#define FANOTIFY_FAN_REPORT_FID_ERR_MSG(res_func, fail) do { \
	if (fail == -1) \
		res_func(TCONF, "FAN_REPORT_FID not supported in kernel?"); \
	if (fail == -2) \
		res_func(TCONF, ...); \
	} while (0)

> +#define FANOTIFY_FAN_REPORT_FID_ERR_MSG(fail) \
> +	fanotify_fan_report_fid_err_msg(__FILE__, __LINE__, tst_res_, (fail))
> +
> +static inline void require_fanotify_fan_report_fid_supported_on_fs(const char *file,
> +	const int lineno, const char *fname)
> +{
> +	int rval;
> +
> +	rval = fanotify_fan_report_fid_supported_on_fs(fname);
> +	if (rval)
> +		fanotify_fan_report_fid_err_msg(file, lineno, tst_brk_, rval);

We don't have to do the if here, just pass the rval, it will not trigger
tst_brk() if we pass zero.


The rest is good.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 05/10] fanotify16: Test also on FUSE
  2020-12-01 17:42 ` [LTP] [PATCH v5 05/10] fanotify16: Test also on FUSE Petr Vorel
@ 2020-12-02 15:46   ` Cyril Hrubis
  0 siblings, 0 replies; 30+ messages in thread
From: Cyril Hrubis @ 2020-12-02 15:46 UTC (permalink / raw)
  To: ltp

Hi!

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>


-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 06/10] fanotify: Add helper for mark support check
  2020-12-01 17:42 ` [LTP] [PATCH v5 06/10] fanotify: Add helper for mark support check Petr Vorel
  2020-12-02  6:27   ` Amir Goldstein
@ 2020-12-02 15:53   ` Cyril Hrubis
  1 sibling, 0 replies; 30+ messages in thread
From: Cyril Hrubis @ 2020-12-02 15:53 UTC (permalink / raw)
  To: ltp

Hi!

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 07/10] fanotify: Introduce SAFE_FANOTIFY_MARK() macro
  2020-12-01 17:42 ` [LTP] [PATCH v5 07/10] fanotify: Introduce SAFE_FANOTIFY_MARK() macro Petr Vorel
@ 2020-12-02 15:55   ` Cyril Hrubis
  2020-12-02 15:58     ` Cyril Hrubis
  0 siblings, 1 reply; 30+ messages in thread
From: Cyril Hrubis @ 2020-12-02 15:55 UTC (permalink / raw)
  To: ltp

Hi!
> +static inline int safe_fanotify_mark(const char *file, const int lineno,
> +			int fd, unsigned int flags, uint64_t mask,
> +			int dfd, const char *pathname)
> +{
> +	int rval;
> +
> +	rval = fanotify_mark(fd, flags, mask, dfd, pathname);
> +
> +	if (rval == -1) {
> +		tst_brk_(file, lineno, TBROK | TERRNO, "fanotify_mark() failed");

I would be a bit more verbose here, i.e. print the mask, pathname and
file descriptors as well.

Otherwise:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>


-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 08/10] fanotify: Use tst_brk_ in safe_fanotify_init()
  2020-12-01 17:42 ` [LTP] [PATCH v5 08/10] fanotify: Use tst_brk_ in safe_fanotify_init() Petr Vorel
@ 2020-12-02 15:55   ` Cyril Hrubis
  0 siblings, 0 replies; 30+ messages in thread
From: Cyril Hrubis @ 2020-12-02 15:55 UTC (permalink / raw)
  To: ltp

Hi!

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>


-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 09/10] fanotify: Add a pedantic check for return value
  2020-12-01 17:42 ` [LTP] [PATCH v5 09/10] fanotify: Add a pedantic check for return value Petr Vorel
@ 2020-12-02 15:56   ` Cyril Hrubis
  0 siblings, 0 replies; 30+ messages in thread
From: Cyril Hrubis @ 2020-12-02 15:56 UTC (permalink / raw)
  To: ltp

Hi!

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>


-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 06/10] fanotify: Add helper for mark support check
  2020-12-02  6:27   ` Amir Goldstein
@ 2020-12-02 15:57     ` Cyril Hrubis
  2020-12-02 17:05     ` Petr Vorel
  1 sibling, 0 replies; 30+ messages in thread
From: Cyril Hrubis @ 2020-12-02 15:57 UTC (permalink / raw)
  To: ltp

Hi!
> Patch set is super! it's the only issue I found, so I don't think you
> need to re-post for that.

Agree, great cleanup, the few minor issues could be dealt with before
pusthing the patches.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 07/10] fanotify: Introduce SAFE_FANOTIFY_MARK() macro
  2020-12-02 15:55   ` Cyril Hrubis
@ 2020-12-02 15:58     ` Cyril Hrubis
  0 siblings, 0 replies; 30+ messages in thread
From: Cyril Hrubis @ 2020-12-02 15:58 UTC (permalink / raw)
  To: ltp

Hi!
> > +static inline int safe_fanotify_mark(const char *file, const int lineno,
> > +			int fd, unsigned int flags, uint64_t mask,
> > +			int dfd, const char *pathname)
> > +{
> > +	int rval;
> > +
> > +	rval = fanotify_mark(fd, flags, mask, dfd, pathname);
> > +
> > +	if (rval == -1) {
> > +		tst_brk_(file, lineno, TBROK | TERRNO, "fanotify_mark() failed");
> 
> I would be a bit more verbose here, i.e. print the mask, pathname and
> file descriptors as well.

Looks like the rest of the safe helpers are not verbose either, let's
push this as it is and we can make them more vebose later on in a
follow up patch.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH] fanotify: Cleanup <sys/fanotify.h> use
  2020-12-01 17:42 ` [LTP] [PATCH] fanotify: Cleanup <sys/fanotify.h> use Petr Vorel
@ 2020-12-02 16:12   ` Cyril Hrubis
  0 siblings, 0 replies; 30+ messages in thread
From: Cyril Hrubis @ 2020-12-02 16:12 UTC (permalink / raw)
  To: ltp

Hi!
Looks reasonable as well.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>


-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 06/10] fanotify: Add helper for mark support check
  2020-12-02  6:27   ` Amir Goldstein
  2020-12-02 15:57     ` Cyril Hrubis
@ 2020-12-02 17:05     ` Petr Vorel
  1 sibling, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-02 17:05 UTC (permalink / raw)
  To: ltp

Hi Amir, Cyril,

> On Tue, Dec 1, 2020 at 7:42 PM Petr Vorel <pvorel@suse.cz> wrote:

> > i.e. FAN_MARK_FILESYSTEM and use it in relevant tests setup().
> > The purpose is to reduce checks (cleanup).

> > NOTE: all tests check only for FAN_MARK_FILESYSTEM, but keep helper
> > generic for future use.

> > Suggested-by: Amir Goldstein <amir73il@gmail.com>
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > New in v5.

> >  testcases/kernel/syscalls/fanotify/fanotify.h | 21 ++++++++++++++++
> >  .../kernel/syscalls/fanotify/fanotify01.c     | 12 +++++----
> >  .../kernel/syscalls/fanotify/fanotify03.c     | 25 +++++++++----------
> >  .../kernel/syscalls/fanotify/fanotify10.c     | 14 +++++------
> >  .../kernel/syscalls/fanotify/fanotify13.c     | 15 +++++------
> >  5 files changed, 55 insertions(+), 32 deletions(-)

> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> > index 821e6cb29..2275a1da3 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> > @@ -351,4 +351,25 @@ static inline void require_fanotify_fan_report_fid_supported_on_fs(const char *f
> >  #define REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(fname) \
> >         require_fanotify_fan_report_fid_supported_on_fs(__FILE__, __LINE__, (fname))

> > +static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
> > +{
> > +       int fd;
> > +       int rval = 0;
> > +
> > +       fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
> > +
> > +       if (fanotify_mark(fd, FAN_MARK_ADD | flag, FAN_ACCESS_PERM, AT_FDCWD, ".") < 0) {
> > +               if (errno == EINVAL) {
> > +                       rval = -1;
> > +               } else {
> > +                       tst_brk(TBROK | TERRNO,
> > +                               "fanotify_mark (%d, FAN_MARK_ADD, ..., FAN_ACCESS_PERM, AT_FDCWD, \".\") failed", fd);
> > +               }

> You meant FAN_ACCESS.

> FAN_ACCESS_PERM requires CONFIG_FANOTIFY_ACCESS_PERMISSIONS which is not
> a requirement for most of the affected tests.
Thanks for explanation, I was thinking about it :).

> Patch set is super! it's the only issue I found, so I don't think you
> need to re-post for that.

Fixing this and Cyril notes in 04/10 and merging tonight.

Kind regards,
Petr

> Thanks,
> Amir.

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 04/10] fanotify: Add helper for FAN_REPORT_FID support on fs
  2020-12-02 15:46   ` Cyril Hrubis
@ 2020-12-02 18:04     ` Petr Vorel
  2020-12-04 10:11       ` Amir Goldstein
  0 siblings, 1 reply; 30+ messages in thread
From: Petr Vorel @ 2020-12-02 18:04 UTC (permalink / raw)
  To: ltp

Hi Amir, Cyril,

> > +	if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS, AT_FDCWD, fname) < 0) {
> > +		if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV) {
> > +			rval = -2;
> > +		} else {
> > +			tst_brk(TBROK | TERRNO,
> > +				"fanotify_mark (%d, FAN_MARK_ADD, ..., AT_FDCWD, \".\") failed", fd);
>                                                                                    ^
> 										   fname?
...

> Maybe this would be just easier to read as a macro:

> #define FANOTIFY_FAN_REPORT_FID_ERR_MSG(res_func, fail) do { \
> 	if (fail == -1) \
> 		res_func(TCONF, "FAN_REPORT_FID not supported in kernel?"); \
> 	if (fail == -2) \
> 		res_func(TCONF, ...); \
> 	} while (0)

...
> > +static inline void require_fanotify_fan_report_fid_supported_on_fs(const char *file,
> > +	const int lineno, const char *fname)
> > +{
> > +	int rval;
> > +
> > +	rval = fanotify_fan_report_fid_supported_on_fs(fname);
> > +	if (rval)
> > +		fanotify_fan_report_fid_err_msg(file, lineno, tst_brk_, rval);

> We don't have to do the if here, just pass the rval, it will not trigger
> tst_brk() if we pass zero.


> The rest is good.

Both your comments fixed, patchset merged.

Thank you both for your patient review,

Amir, I'm sorry it took me that long.
Looking forward to your patchset :).

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 04/10] fanotify: Add helper for FAN_REPORT_FID support on fs
  2020-12-02 18:04     ` Petr Vorel
@ 2020-12-04 10:11       ` Amir Goldstein
  2020-12-04 13:47         ` Petr Vorel
  0 siblings, 1 reply; 30+ messages in thread
From: Amir Goldstein @ 2020-12-04 10:11 UTC (permalink / raw)
  To: ltp

On Wed, Dec 2, 2020 at 8:04 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Amir, Cyril,
>
> > > +   if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS, AT_FDCWD, fname) < 0) {
> > > +           if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV) {
> > > +                   rval = -2;
> > > +           } else {
> > > +                   tst_brk(TBROK | TERRNO,
> > > +                           "fanotify_mark (%d, FAN_MARK_ADD, ..., AT_FDCWD, \".\") failed", fd);
> >                                                                                    ^
> >                                                                                  fname?
> ...
>
> > Maybe this would be just easier to read as a macro:
>
> > #define FANOTIFY_FAN_REPORT_FID_ERR_MSG(res_func, fail) do { \
> >       if (fail == -1) \
> >               res_func(TCONF, "FAN_REPORT_FID not supported in kernel?"); \
> >       if (fail == -2) \
> >               res_func(TCONF, ...); \
> >       } while (0)
>

Sorry, I ended up reverting that back to a function.

> ...
> > > +static inline void require_fanotify_fan_report_fid_supported_on_fs(const char *file,
> > > +   const int lineno, const char *fname)
> > > +{
> > > +   int rval;
> > > +
> > > +   rval = fanotify_fan_report_fid_supported_on_fs(fname);
> > > +   if (rval)
> > > +           fanotify_fan_report_fid_err_msg(file, lineno, tst_brk_, rval);
>
> > We don't have to do the if here, just pass the rval, it will not trigger
> > tst_brk() if we pass zero.
>
>
> > The rest is good.
>
> Both your comments fixed, patchset merged.
>
> Thank you both for your patient review,
>
> Amir, I'm sorry it took me that long.

No worries.
I know there was a lot of black magic behind all the flag checks
that needed explanations.

> Looking forward to your patchset :).
>

Posted.
Note that I only tested upstream, v5.8, v4.20 and not kernels
without permission events support and not exfat.

I trust you will help to fill those testing gaps.

If I am not mistaken, before my fixes, fanotify10 was going to fail
instead of TCONF on exfat/ntfs and kernel >= v5.9, but I did not check.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [LTP] [PATCH v5 04/10] fanotify: Add helper for FAN_REPORT_FID support on fs
  2020-12-04 10:11       ` Amir Goldstein
@ 2020-12-04 13:47         ` Petr Vorel
  0 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-04 13:47 UTC (permalink / raw)
  To: ltp

Hi Amir,

...
> > > Maybe this would be just easier to read as a macro:

> > > #define FANOTIFY_FAN_REPORT_FID_ERR_MSG(res_func, fail) do { \
> > >       if (fail == -1) \
> > >               res_func(TCONF, "FAN_REPORT_FID not supported in kernel?"); \
> > >       if (fail == -2) \
> > >               res_func(TCONF, ...); \
> > >       } while (0)


> Sorry, I ended up reverting that back to a function.
np, probably better anyway.

...
> > > The rest is good.

> > Both your comments fixed, patchset merged.

> > Thank you both for your patient review,

> > Amir, I'm sorry it took me that long.

> No worries.
> I know there was a lot of black magic behind all the flag checks
> that needed explanations.

> > Looking forward to your patchset :).


> Posted.
Thanks!

> Note that I only tested upstream, v5.8, v4.20 and not kernels
> without permission events support and not exfat.

> I trust you will help to fill those testing gaps.
Sure! I should get into that early next week.

> If I am not mistaken, before my fixes, fanotify10 was going to fail
> instead of TCONF on exfat/ntfs and kernel >= v5.9, but I did not check.
Don't fail on master on my openSUSE kernel 5.10.0-rc5-2.ge84a0b5-default

> Thanks,
> Amir.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 30+ messages in thread
end of thread, other threads:[~2020-12-04 13:47 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-01 17:42 [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
2020-12-01 17:42 ` [LTP] [PATCH v5 01/10] fanotify12: Drop incorrect hint Petr Vorel
2020-12-02 15:09   ` Cyril Hrubis
2020-12-01 17:42 ` [LTP] [PATCH v5 02/10] fanotify: Add helper for access permission check Petr Vorel
2020-12-02 15:24   ` Cyril Hrubis
2020-12-01 17:42 ` [LTP] [PATCH v5 03/10] fanotify: Add helper for event support check Petr Vorel
2020-12-02 15:36   ` Cyril Hrubis
2020-12-01 17:42 ` [LTP] [PATCH v5 04/10] fanotify: Add helper for FAN_REPORT_FID support on fs Petr Vorel
2020-12-02 15:46   ` Cyril Hrubis
2020-12-02 18:04     ` Petr Vorel
2020-12-04 10:11       ` Amir Goldstein
2020-12-04 13:47         ` Petr Vorel
2020-12-01 17:42 ` [LTP] [PATCH v5 05/10] fanotify16: Test also on FUSE Petr Vorel
2020-12-02 15:46   ` Cyril Hrubis
2020-12-01 17:42 ` [LTP] [PATCH v5 06/10] fanotify: Add helper for mark support check Petr Vorel
2020-12-02  6:27   ` Amir Goldstein
2020-12-02 15:57     ` Cyril Hrubis
2020-12-02 17:05     ` Petr Vorel
2020-12-02 15:53   ` Cyril Hrubis
2020-12-01 17:42 ` [LTP] [PATCH v5 07/10] fanotify: Introduce SAFE_FANOTIFY_MARK() macro Petr Vorel
2020-12-02 15:55   ` Cyril Hrubis
2020-12-02 15:58     ` Cyril Hrubis
2020-12-01 17:42 ` [LTP] [PATCH v5 08/10] fanotify: Use tst_brk_ in safe_fanotify_init() Petr Vorel
2020-12-02 15:55   ` Cyril Hrubis
2020-12-01 17:42 ` [LTP] [PATCH v5 09/10] fanotify: Add a pedantic check for return value Petr Vorel
2020-12-02 15:56   ` Cyril Hrubis
2020-12-01 17:42 ` [LTP] [PATCH] fanotify: Cleanup <sys/fanotify.h> use Petr Vorel
2020-12-02 16:12   ` Cyril Hrubis
2020-12-01 17:46 ` [LTP] [PATCH v5 00/10] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
2020-12-02  6:47 ` Amir Goldstein

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.