All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 0/5] Fanotify cleanup and test for v5.9 regression
@ 2020-12-04  9:59 Amir Goldstein
  2020-12-04  9:59 ` [LTP] [PATCH 1/5] syscalls/fanotify: Generalize check for FAN_REPORT_FID support Amir Goldstein
                   ` (5 more replies)
  0 siblings, 6 replies; 23+ messages in thread
From: Amir Goldstein @ 2020-12-04  9:59 UTC (permalink / raw)
  To: ltp

Hi Petr,

Here are my followup patches to your great cleanup and
a new test for a bug fix that is already included in v5.9.y.

Note that I resurrected the helper from your V5 patches that
you turned into a macro following Cyril's comment.
I hope you both find the result satisfying.

There are no direct calls to fanotify_init() after those
cleanups (except for the intended use in fanotify14).

FYI, I have another test (inotify) for another v5.9 regression.
The fix is queued for upstream, but did reach upstream yet, so
I will post that test later.

Thanks,
Amir.

Amir Goldstein (5):
  syscalls/fanotify: Generalize check for FAN_REPORT_FID support
  syscalls/fanotify: Use generic checks for fanotify_init flags
  syscalls/fanotify09: Read variable length events
  syscalls/fanotify09: Add test case with two non-dir children
  syscalls/fanotify09: Add test case for events with filename info

 testcases/kernel/syscalls/fanotify/fanotify.h |  46 +++--
 .../kernel/syscalls/fanotify/fanotify01.c     |   4 +-
 .../kernel/syscalls/fanotify/fanotify09.c     | 167 +++++++++++++-----
 .../kernel/syscalls/fanotify/fanotify10.c     |  26 ++-
 .../kernel/syscalls/fanotify/fanotify11.c     |  21 +--
 .../kernel/syscalls/fanotify/fanotify13.c     |   2 +-
 .../kernel/syscalls/fanotify/fanotify15.c     |   2 +-
 .../kernel/syscalls/fanotify/fanotify16.c     |  14 +-
 8 files changed, 180 insertions(+), 102 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 1/5] syscalls/fanotify: Generalize check for FAN_REPORT_FID support
  2020-12-04  9:59 [LTP] [PATCH 0/5] Fanotify cleanup and test for v5.9 regression Amir Goldstein
@ 2020-12-04  9:59 ` Amir Goldstein
  2020-12-04 13:52   ` Petr Vorel
  2020-12-07 10:48   ` Jan Kara
  2020-12-04  9:59 ` [LTP] [PATCH 2/5] syscalls/fanotify: Use generic checks for fanotify_init flags Amir Goldstein
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 23+ messages in thread
From: Amir Goldstein @ 2020-12-04  9:59 UTC (permalink / raw)
  To: ltp

Generalize the helpers to be able to check any fanotify_init() flags
and pass FAN_REPORT_FID as argument in call sites.

Add helper fanotify_init_flags_supported_by_kernel() to check for
kernel support for fanotify_init flags without checking fs support
for those flags.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 testcases/kernel/syscalls/fanotify/fanotify.h | 46 ++++++++++++-------
 .../kernel/syscalls/fanotify/fanotify01.c     |  4 +-
 .../kernel/syscalls/fanotify/fanotify13.c     |  2 +-
 .../kernel/syscalls/fanotify/fanotify15.c     |  2 +-
 .../kernel/syscalls/fanotify/fanotify16.c     |  2 +-
 5 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 82e03db26..8907db052 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -286,15 +286,15 @@ static inline int fanotify_events_supported_by_kernel(uint64_t mask)
 
 /*
  * @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
+ * @return -1: @flags not supported in kernel
+ * @return -2: @flags not supported on tested filesystem (tested if @fname is not NULL)
  */
-static inline int fanotify_fan_report_fid_supported_on_fs(const char *fname)
+static inline int fanotify_init_flags_supported_on_fs(unsigned int flags, const char *fname)
 {
 	int fd;
 	int rval = 0;
 
-	fd = fanotify_init(FAN_CLASS_NOTIF | FAN_REPORT_FID, O_RDONLY);
+	fd = fanotify_init(flags, O_RDONLY);
 
 	if (fd < 0) {
 		if (errno == ENOSYS)
@@ -306,7 +306,7 @@ static inline int fanotify_fan_report_fid_supported_on_fs(const char *fname)
 		tst_brk(TBROK | TERRNO, "fanotify_init() failed");
 	}
 
-	if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS, AT_FDCWD, fname) < 0) {
+	if (fname && fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS, AT_FDCWD, fname) < 0) {
 		if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV) {
 			rval = -2;
 		} else {
@@ -321,20 +321,32 @@ static inline int fanotify_fan_report_fid_supported_on_fs(const char *fname)
 	return rval;
 }
 
-#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, "FAN_REPORT_FID not supported on %s filesystem", \
-			tst_device->fs_type); \
-	} while (0)
+static inline int fanotify_init_flags_supported_by_kernel(unsigned int flags)
+{
+	return fanotify_init_flags_supported_on_fs(flags, NULL);
+}
+
+typedef void (*tst_res_func_t)(const char *file, const int lineno,
+			       int ttype, const char *fmt, ...);
+
+static inline void fanotify_init_flags_err_msg(const char *flags_str,
+	const char *file, const int lineno, tst_res_func_t res_func, int fail)
+{
+	if (fail == -1)
+		res_func(file, lineno, TCONF,
+			 "%s not supported in kernel?", flags_str);
+	if (fail == -2)
+		res_func(file, lineno, TCONF,
+			 "%s not supported on %s filesystem",
+			 flags_str, tst_device->fs_type);
+}
 
-#define FANOTIFY_FAN_REPORT_FID_ERR_MSG(fail) \
-	FANOTIFY_FAN_REPORT_FID_ERR_MSG_(tst_res, (fail))
+#define FANOTIFY_INIT_FLAGS_ERR_MSG(flags, fail) \
+	fanotify_init_flags_err_msg(#flags, __FILE__, __LINE__, tst_res_, (fail))
 
-#define REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(fname) do { \
-	FANOTIFY_FAN_REPORT_FID_ERR_MSG_(tst_brk, \
-		fanotify_fan_report_fid_supported_on_fs(fname)); \
+#define REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(flags, fname) do { \
+	fanotify_init_flags_err_msg(#flags, __FILE__, __LINE__, tst_brk_, \
+		fanotify_init_flags_supported_on_fs(flags, fname)); \
 	} while (0)
 
 static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
index d6d72dad9..cdb01730f 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify01.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
@@ -90,7 +90,7 @@ static void test_fanotify(unsigned int n)
 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
 
 	if (fan_report_fid_unsupported && (tc->init_flags & FAN_REPORT_FID)) {
-		FANOTIFY_FAN_REPORT_FID_ERR_MSG(fan_report_fid_unsupported);
+		FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_FID, fan_report_fid_unsupported);
 		return;
 	}
 
@@ -334,7 +334,7 @@ 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);
+	fan_report_fid_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_FID, fname);
 	filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
 }
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index a402cdb13..c9cf10555 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -256,7 +256,7 @@ out:
 
 static void do_setup(void)
 {
-	REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(MOUNT_PATH);
+	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, MOUNT_PATH);
 
 	filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify15.c b/testcases/kernel/syscalls/fanotify/fanotify15.c
index 9e3748bc2..ba8259c7c 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify15.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify15.c
@@ -270,7 +270,7 @@ static void do_test(unsigned int number)
 static void do_setup(void)
 {
 	SAFE_MKDIR(TEST_DIR, 0755);
-	REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(TEST_DIR);
+	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, TEST_DIR);
 	fanotify_fd = SAFE_FANOTIFY_INIT(FAN_REPORT_FID, O_RDONLY);
 }
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
index a554c7d39..a4409df14 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify16.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify16.c
@@ -551,7 +551,7 @@ check_match:
 
 static void setup(void)
 {
-	REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(MOUNT_PATH);
+	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, MOUNT_PATH);
 
 	sprintf(dname1, "%s/%s", MOUNT_PATH, DIR_NAME1);
 	sprintf(dname2, "%s/%s", MOUNT_PATH, DIR_NAME2);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 2/5] syscalls/fanotify: Use generic checks for fanotify_init flags
  2020-12-04  9:59 [LTP] [PATCH 0/5] Fanotify cleanup and test for v5.9 regression Amir Goldstein
  2020-12-04  9:59 ` [LTP] [PATCH 1/5] syscalls/fanotify: Generalize check for FAN_REPORT_FID support Amir Goldstein
@ 2020-12-04  9:59 ` Amir Goldstein
  2020-12-04 13:55   ` Petr Vorel
  2020-12-07 10:52   ` Jan Kara
  2020-12-04  9:59 ` [LTP] [PATCH 3/5] syscalls/fanotify09: Read variable length events Amir Goldstein
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 23+ messages in thread
From: Amir Goldstein @ 2020-12-04  9:59 UTC (permalink / raw)
  To: ltp

Convert remaining tests to SAFE_FANOTIFY_INIT and use the generic
helpers to check requires kernel/fs support for fanotify_init flags
in advance.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/fanotify/fanotify10.c     | 26 ++++++++-----------
 .../kernel/syscalls/fanotify/fanotify11.c     | 21 ++++++---------
 .../kernel/syscalls/fanotify/fanotify16.c     | 14 ++--------
 3 files changed, 21 insertions(+), 40 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index 404e57daa..cc164359f 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -57,6 +57,7 @@ static unsigned int fanotify_class[] = {
 	FAN_REPORT_DFID_NAME_FID,
 };
 #define NUM_CLASSES ARRAY_SIZE(fanotify_class)
+#define NUM_PRIORITIES 3
 
 #define GROUPS_PER_PRIO 3
 
@@ -64,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 fan_report_dfid_unsupported;
 static int filesystem_mark_unsupported;
 
 #define MOUNT_PATH "fs_mnt"
@@ -294,21 +296,8 @@ static int create_fanotify_groups(unsigned int n)
 
 	for (p = 0; p < num_classes; p++) {
 		for (i = 0; i < GROUPS_PER_PRIO; i++) {
-			fd_notify[p][i] = fanotify_init(fanotify_class[p] |
-							FAN_NONBLOCK, O_RDONLY);
-			if (fd_notify[p][i] == -1) {
-				if (errno == EINVAL &&
-				    fanotify_class[p] & FAN_REPORT_NAME) {
-					tst_res(TCONF,
-						"FAN_REPORT_NAME not supported by kernel?");
-					/* Do not try creating this group again */
-					num_classes--;
-					return -1;
-				}
-
-				tst_brk(TBROK | TERRNO,
-					"fanotify_init(%x, 0) failed", fanotify_class[p]);
-			}
+			fd_notify[p][i] = SAFE_FANOTIFY_INIT(fanotify_class[p] |
+							     FAN_NONBLOCK, O_RDONLY);
 
 			/*
 			 * Add mark for each group.
@@ -518,6 +507,13 @@ 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);
+	fan_report_dfid_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_DFID_NAME,
+									  MOUNT_PATH);
+	if (fan_report_dfid_unsupported) {
+		FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_DFID_NAME, fan_report_dfid_unsupported);
+		/* Limit tests to legacy priority classes */
+		num_classes = NUM_PRIORITIES;
+	}
 
 	/* Create another bind mount@another path for generating events */
 	SAFE_MKDIR(MNT2_PATH, 0755);
diff --git a/testcases/kernel/syscalls/fanotify/fanotify11.c b/testcases/kernel/syscalls/fanotify/fanotify11.c
index 785b5c5a5..f3b60cecb 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify11.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify11.c
@@ -36,6 +36,8 @@
 #define gettid() syscall(SYS_gettid)
 static int tid;
 
+static int fan_report_tid_unsupported;
+
 void *thread_create_file(void *arg LTP_ATTRIBUTE_UNUSED)
 {
 	char tid_file[64] = {0};
@@ -63,17 +65,13 @@ void test01(unsigned int i)
 			i, (tcases[i] & FAN_REPORT_TID) ? "with" : "without",
 			tgid, tid, event.pid);
 
-	fd_notify = fanotify_init(tcases[i], 0);
-	if (fd_notify < 0) {
-		if (errno == EINVAL && (tcases[i] & FAN_REPORT_TID)) {
-			tst_res(TCONF,
-				"FAN_REPORT_TID not supported in kernel?");
-			return;
-		}
-		tst_brk(TBROK | TERRNO, "fanotify_init(0x%x, 0) failed",
-				tcases[i]);
+	if (fan_report_tid_unsupported && (tcases[i] & FAN_REPORT_TID)) {
+		FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_TID, fan_report_tid_unsupported);
+		return;
 	}
 
+	fd_notify = SAFE_FANOTIFY_INIT(tcases[i], 0);
+
 	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD,
 			FAN_ALL_EVENTS | FAN_EVENT_ON_CHILD, AT_FDCWD, ".");
 
@@ -96,10 +94,7 @@ void test01(unsigned int i)
 
 static void setup(void)
 {
-	int fd;
-
-	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
-	SAFE_CLOSE(fd);
+	fan_report_tid_unsupported = fanotify_init_flags_supported_by_kernel(FAN_REPORT_TID);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
index a4409df14..5ffaec92f 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify16.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify16.c
@@ -158,17 +158,7 @@ static void do_test(unsigned int number)
 
 	tst_res(TINFO, "Test #%d: %s", number, tc->tname);
 
-	fd_notify = fanotify_init(group->flag, 0);
-	if (fd_notify == -1) {
-		if (errno == EINVAL) {
-			tst_res(TCONF,
-				"%s not supported by kernel", group->name);
-			return;
-		}
-
-		tst_brk(TBROK | TERRNO,
-			"fanotify_init(%s, 0) failed", group->name);
-	}
+	fd_notify = SAFE_FANOTIFY_INIT(group->flag, 0);
 
 	/*
 	 * Watch dir modify events with name in filesystem/dir
@@ -551,7 +541,7 @@ check_match:
 
 static void setup(void)
 {
-	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, MOUNT_PATH);
+	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_DIR_FID, MOUNT_PATH);
 
 	sprintf(dname1, "%s/%s", MOUNT_PATH, DIR_NAME1);
 	sprintf(dname2, "%s/%s", MOUNT_PATH, DIR_NAME2);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 3/5] syscalls/fanotify09: Read variable length events
  2020-12-04  9:59 [LTP] [PATCH 0/5] Fanotify cleanup and test for v5.9 regression Amir Goldstein
  2020-12-04  9:59 ` [LTP] [PATCH 1/5] syscalls/fanotify: Generalize check for FAN_REPORT_FID support Amir Goldstein
  2020-12-04  9:59 ` [LTP] [PATCH 2/5] syscalls/fanotify: Use generic checks for fanotify_init flags Amir Goldstein
@ 2020-12-04  9:59 ` Amir Goldstein
  2020-12-04 14:16   ` Petr Vorel
                     ` (2 more replies)
  2020-12-04  9:59 ` [LTP] [PATCH 4/5] syscalls/fanotify09: Add test case with two non-dir children Amir Goldstein
                   ` (2 subsequent siblings)
  5 siblings, 3 replies; 23+ messages in thread
From: Amir Goldstein @ 2020-12-04  9:59 UTC (permalink / raw)
  To: ltp

In preparation of testing events with filename info, teach the
how to read variable length events and parse the name info.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/fanotify/fanotify09.c     | 88 +++++++++++++------
 1 file changed, 60 insertions(+), 28 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
index daeb712d2..7bb901cf3 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify09.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
@@ -138,42 +138,73 @@ static void cleanup_fanotify_groups(void)
 }
 
 static void event_res(int ttype, int group,
-		      struct fanotify_event_metadata *event)
+		      struct fanotify_event_metadata *event,
+		      const char *filename)
 {
-	int len;
-	sprintf(symlnk, "/proc/self/fd/%d", event->fd);
-	len = readlink(symlnk, fdpath, sizeof(fdpath));
-	if (len < 0)
-		len = 0;
-	fdpath[len] = 0;
-	tst_res(ttype, "group %d got event: mask %llx pid=%u fd=%d path=%s",
+	if (event->fd != FAN_NOFD) {
+		int len = 0;
+		sprintf(symlnk, "/proc/self/fd/%d", event->fd);
+		len = readlink(symlnk, fdpath, sizeof(fdpath));
+		if (len < 0)
+			len = 0;
+		fdpath[len] = 0;
+		filename = fdpath;
+	}
+	tst_res(ttype, "group %d got event: mask %llx pid=%u fd=%d filename=%s",
 		group, (unsigned long long)event->mask,
-		(unsigned)event->pid, event->fd, fdpath);
+		(unsigned)event->pid, event->fd, filename);
+}
+
+static const char *event_filename(struct fanotify_event_metadata *event)
+{
+	struct fanotify_event_info_fid *event_fid;
+	struct file_handle *file_handle;
+	const char *filename, *end;
+
+	if (event->event_len <= FAN_EVENT_METADATA_LEN)
+		return "";
+
+	event_fid = (struct fanotify_event_info_fid *)(event + 1);
+	file_handle = (struct file_handle *)event_fid->handle;
+	filename = (char *)file_handle->f_handle + file_handle->handle_bytes;
+	end = (char *)event_fid + event_fid->hdr.len;
+
+	/* End of event_fid could have name, zero padding, both or none */
+	return (filename == end) ? "" : filename;
 }
 
 static void verify_event(int group, struct fanotify_event_metadata *event,
-			 uint32_t expect)
+			 uint32_t expect, const char *expect_filename)
 {
+	const char *filename = event_filename(event);
+
 	if (event->mask != expect) {
 		tst_res(TFAIL, "group %d got event: mask %llx (expected %llx) "
-			"pid=%u fd=%d", group, (unsigned long long)event->mask,
+			"pid=%u fd=%d filename=%s", group, (unsigned long long)event->mask,
 			(unsigned long long)expect,
-			(unsigned)event->pid, event->fd);
+			(unsigned)event->pid, event->fd, filename);
 	} else if (event->pid != getpid()) {
 		tst_res(TFAIL, "group %d got event: mask %llx pid=%u "
-			"(expected %u) fd=%d", group,
+			"(expected %u) fd=%d filename=%s", group,
 			(unsigned long long)event->mask, (unsigned)event->pid,
-			(unsigned)getpid(), event->fd);
+			(unsigned)getpid(), event->fd, filename);
+	} else if (strcmp(filename, expect_filename)) {
+		tst_res(TFAIL, "group %d got event: mask %llx pid=%u "
+			"fd=%d filename='%s' (expected '%s')", group,
+			(unsigned long long)event->mask, (unsigned)event->pid,
+			event->fd, filename, expect_filename);
 	} else {
-		event_res(TPASS, group, event);
+		event_res(TPASS, group, event, filename);
 	}
+	if (event->fd != FAN_NOFD)
+		SAFE_CLOSE(event->fd);
 }
 
 static void test_fanotify(unsigned int n)
 {
 	int ret, dirfd;
 	unsigned int i;
-	struct fanotify_event_metadata *event, *ev;
+	struct fanotify_event_metadata *event;
 	struct tcase *tc = &tcases[n];
 
 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
@@ -210,20 +241,21 @@ static void test_fanotify(unsigned int n)
 			ret, tc->nevents * (int)FAN_EVENT_METADATA_LEN);
 	}
 	event = (struct fanotify_event_metadata *)event_buf;
-	verify_event(0, event, FAN_MODIFY);
-	if (tc->ondir)
-		verify_event(0, event + 1, FAN_CLOSE_NOWRITE);
-	if (ret > tc->nevents * (int)FAN_EVENT_METADATA_LEN) {
+	verify_event(0, event, FAN_MODIFY, "");
+	event = FAN_EVENT_NEXT(event, ret);
+	if (tc->ondir) {
+		verify_event(0, event, FAN_CLOSE_NOWRITE, "");
+		event = FAN_EVENT_NEXT(event, ret);
+	}
+	if (ret > 0) {
 		tst_res(TFAIL,
-			"first group got more than %d events (%d > %d)",
-			tc->nevents, ret,
-			tc->nevents * (int)FAN_EVENT_METADATA_LEN);
+			"first group got more than %d events (%d bytes)",
+			tc->nevents, ret);
 	}
 	/* Close all file descriptors of read events */
-	for (ev = event; ret >= (int)FAN_EVENT_METADATA_LEN; ev++) {
-		if (ev->fd != FAN_NOFD)
-			SAFE_CLOSE(ev->fd);
-		ret -= (int)FAN_EVENT_METADATA_LEN;
+	for (; FAN_EVENT_OK(event, ret); FAN_EVENT_NEXT(event, ret)) {
+		if (event->fd != FAN_NOFD)
+			SAFE_CLOSE(event->fd);
 	}
 
 	/*
@@ -233,7 +265,7 @@ static void test_fanotify(unsigned int n)
 	for (i = 1; i < NUM_GROUPS; i++) {
 		ret = read(fd_notify[i], event_buf, FAN_EVENT_METADATA_LEN);
 		if (ret > 0) {
-			event_res(TFAIL, i, event);
+			event_res(TFAIL, i, event, "");
 			if (event->fd != FAN_NOFD)
 				SAFE_CLOSE(event->fd);
 			continue;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 4/5] syscalls/fanotify09: Add test case with two non-dir children
  2020-12-04  9:59 [LTP] [PATCH 0/5] Fanotify cleanup and test for v5.9 regression Amir Goldstein
                   ` (2 preceding siblings ...)
  2020-12-04  9:59 ` [LTP] [PATCH 3/5] syscalls/fanotify09: Read variable length events Amir Goldstein
@ 2020-12-04  9:59 ` Amir Goldstein
  2020-12-04 14:19   ` Petr Vorel
  2020-12-07 11:01   ` Jan Kara
  2020-12-04  9:59 ` [LTP] [PATCH 5/5] syscalls/fanotify09: Add test case for events with filename info Amir Goldstein
  2020-12-04 14:27 ` [LTP] [PATCH 0/5] Fanotify cleanup and test for v5.9 regression Petr Vorel
  5 siblings, 2 replies; 23+ messages in thread
From: Amir Goldstein @ 2020-12-04  9:59 UTC (permalink / raw)
  To: ltp

The existing test cases generate FAN_MODIFY event on a non-dir child
and FAN_CLOSE_NOWRITE event the parent or sibling subdir.

Add a test case that generates FAN_CLOSE_NOWRITE on a sibling non-dir
child.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/fanotify/fanotify09.c     | 38 ++++++++++++++-----
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
index 7bb901cf3..0f1923981 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify09.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
@@ -55,13 +55,14 @@ static char event_buf[EVENT_BUF_LEN];
 
 #define MOUNT_NAME "mntpoint"
 #define DIR_NAME "testdir"
+#define FILE2_NAME "testfile"
 static int mount_created;
 
 static struct tcase {
 	const char *tname;
 	struct fanotify_mark_type mark;
 	unsigned int ondir;
-	const char *testdir;
+	const char *close_nowrite;
 	int nevents;
 } tcases[] = {
 	{
@@ -92,6 +93,13 @@ static struct tcase {
 		DIR_NAME,
 		2,
 	},
+	{
+		"Events on non-dir children with both parent and mount marks",
+		INIT_FANOTIFY_MARK_TYPE(MOUNT),
+		0,
+		FILE2_NAME,
+		2,
+	},
 };
 
 static void create_fanotify_groups(struct tcase *tc)
@@ -112,7 +120,7 @@ static void create_fanotify_groups(struct tcase *tc)
 		SAFE_FANOTIFY_MARK(fd_notify[i],
 				    FAN_MARK_ADD | mark->flag,
 				    FAN_CLOSE_NOWRITE | onchild,
-				    AT_FDCWD, tc->testdir);
+				    AT_FDCWD, tc->close_nowrite);
 
 		/*
 		 * Add inode mark on parent for each group with MODIFY event,
@@ -216,9 +224,9 @@ static void test_fanotify(unsigned int n)
 	 */
 	SAFE_FILE_PRINTF(fname, "1");
 	/*
-	 * generate FAN_CLOSE_NOWRITE event on a testdir (subdir or ".")
+	 * generate FAN_CLOSE_NOWRITE event on a child, subdir or "."
 	 */
-	dirfd = SAFE_OPEN(tc->testdir, O_RDONLY);
+	dirfd = SAFE_OPEN(tc->close_nowrite, O_RDONLY);
 	if (dirfd >= 0)
 		SAFE_CLOSE(dirfd);
 
@@ -243,7 +251,7 @@ static void test_fanotify(unsigned int n)
 	event = (struct fanotify_event_metadata *)event_buf;
 	verify_event(0, event, FAN_MODIFY, "");
 	event = FAN_EVENT_NEXT(event, ret);
-	if (tc->ondir) {
+	if (tc->nevents > 1) {
 		verify_event(0, event, FAN_CLOSE_NOWRITE, "");
 		event = FAN_EVENT_NEXT(event, ret);
 	}
@@ -260,14 +268,23 @@ static void test_fanotify(unsigned int n)
 
 	/*
 	 * Then verify the rest of the groups did not get the MODIFY event and
-	 * did not get the FAN_CLOSE_NOWRITE event on testdir.
+	 * got the FAN_CLOSE_NOWRITE event only on a non-directory.
 	 */
 	for (i = 1; i < NUM_GROUPS; i++) {
-		ret = read(fd_notify[i], event_buf, FAN_EVENT_METADATA_LEN);
+		ret = read(fd_notify[i], event_buf, EVENT_BUF_LEN);
 		if (ret > 0) {
-			event_res(TFAIL, i, event, "");
-			if (event->fd != FAN_NOFD)
-				SAFE_CLOSE(event->fd);
+			uint32_t expect = 0;
+
+			if (tc->nevents > 1 && !tc->ondir)
+				expect = FAN_CLOSE_NOWRITE;
+
+			event = (struct fanotify_event_metadata *)event_buf;
+			verify_event(i, event, expect, "");
+
+			for (; FAN_EVENT_OK(event, ret); FAN_EVENT_NEXT(event, ret)) {
+				if (event->fd != FAN_NOFD)
+					SAFE_CLOSE(event->fd);
+			}
 			continue;
 		}
 
@@ -295,6 +312,7 @@ static void setup(void)
 
 	sprintf(fname, "tfile_%d", getpid());
 	SAFE_FILE_PRINTF(fname, "1");
+	SAFE_FILE_PRINTF(FILE2_NAME, "1");
 }
 
 static void cleanup(void)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 5/5] syscalls/fanotify09: Add test case for events with filename info
  2020-12-04  9:59 [LTP] [PATCH 0/5] Fanotify cleanup and test for v5.9 regression Amir Goldstein
                   ` (3 preceding siblings ...)
  2020-12-04  9:59 ` [LTP] [PATCH 4/5] syscalls/fanotify09: Add test case with two non-dir children Amir Goldstein
@ 2020-12-04  9:59 ` Amir Goldstein
  2020-12-04 14:22   ` Petr Vorel
  2020-12-07 11:11   ` Jan Kara
  2020-12-04 14:27 ` [LTP] [PATCH 0/5] Fanotify cleanup and test for v5.9 regression Petr Vorel
  5 siblings, 2 replies; 23+ messages in thread
From: Amir Goldstein @ 2020-12-04  9:59 UTC (permalink / raw)
  To: ltp

Kernel v5.9 has a bug when there is a mark on filesystem or mount
interested in events with filename AND there is an additional
mark on a parent watching children.

In some situations the event is reported to filesystem or mount mark
without the filename info.

This bug is fixed by kernel commit 7372e79c9eb9:
  fanotify: fix logic of reporting name info with watched parent

The test case requires a blockdev filesystem.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/fanotify/fanotify09.c     | 49 ++++++++++++++++---
 1 file changed, 42 insertions(+), 7 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
index 0f1923981..25b6b8be1 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify09.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
@@ -19,6 +19,10 @@
  * Test case #2 is a regression test for commit 55bf882c7f13:
  *
  *      fanotify: fix merging marks masks with FAN_ONDIR
+ *
+ * Test case #5 is a regression test for commit 7372e79c9eb9:
+ *
+ *      fanotify: fix logic of reporting name info with watched parent
  */
 #define _GNU_SOURCE
 #include "config.h"
@@ -53,15 +57,19 @@ static int fd_notify[NUM_GROUPS];
 
 static char event_buf[EVENT_BUF_LEN];
 
+#define MOUNT_PATH "fs_mnt"
 #define MOUNT_NAME "mntpoint"
 #define DIR_NAME "testdir"
 #define FILE2_NAME "testfile"
 static int mount_created;
 
+static int fan_report_dfid_unsupported;
+
 static struct tcase {
 	const char *tname;
 	struct fanotify_mark_type mark;
 	unsigned int ondir;
+	unsigned int report_name;
 	const char *close_nowrite;
 	int nevents;
 } tcases[] = {
@@ -69,6 +77,7 @@ static struct tcase {
 		"Events on non-dir child with both parent and mount marks",
 		INIT_FANOTIFY_MARK_TYPE(MOUNT),
 		0,
+		0,
 		DIR_NAME,
 		1,
 	},
@@ -76,6 +85,7 @@ static struct tcase {
 		"Events on non-dir child and subdir with both parent and mount marks",
 		INIT_FANOTIFY_MARK_TYPE(MOUNT),
 		FAN_ONDIR,
+		0,
 		DIR_NAME,
 		2,
 	},
@@ -83,6 +93,7 @@ static struct tcase {
 		"Events on non-dir child and parent with both parent and mount marks",
 		INIT_FANOTIFY_MARK_TYPE(MOUNT),
 		FAN_ONDIR,
+		0,
 		".",
 		2,
 	},
@@ -90,6 +101,7 @@ static struct tcase {
 		"Events on non-dir child and subdir with both parent and subdir marks",
 		INIT_FANOTIFY_MARK_TYPE(INODE),
 		FAN_ONDIR,
+		0,
 		DIR_NAME,
 		2,
 	},
@@ -97,6 +109,15 @@ static struct tcase {
 		"Events on non-dir children with both parent and mount marks",
 		INIT_FANOTIFY_MARK_TYPE(MOUNT),
 		0,
+		0,
+		FILE2_NAME,
+		2,
+	},
+	{
+		"Events on non-dir child with both parent and mount marks and filename info",
+		INIT_FANOTIFY_MARK_TYPE(MOUNT),
+		0,
+		FAN_REPORT_DFID_NAME,
 		FILE2_NAME,
 		2,
 	},
@@ -105,12 +126,15 @@ static struct tcase {
 static void create_fanotify_groups(struct tcase *tc)
 {
 	struct fanotify_mark_type *mark = &tc->mark;
-	unsigned int i, onchild, ondir = tc->ondir;
+	unsigned int i, onchild, report_name, ondir = tc->ondir;
 
 	for (i = 0; i < NUM_GROUPS; i++) {
-		fd_notify[i] = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF |
-						  FAN_NONBLOCK,
-						  O_RDONLY);
+		/*
+		 * The first group may request events with filename info.
+		 */
+		report_name = (i == 0) ? tc->report_name : 0;
+		fd_notify[i] = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | report_name |
+						  FAN_NONBLOCK, O_RDONLY);
 
 		/*
 		 * Add subdir or mount mark for each group with CLOSE event,
@@ -217,6 +241,11 @@ static void test_fanotify(unsigned int n)
 
 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
 
+	if (fan_report_dfid_unsupported && tc->report_name) {
+		FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_DFID_NAME, fan_report_dfid_unsupported);
+		return;
+	}
+
 	create_fanotify_groups(tc);
 
 	/*
@@ -249,10 +278,11 @@ static void test_fanotify(unsigned int n)
 			ret, tc->nevents * (int)FAN_EVENT_METADATA_LEN);
 	}
 	event = (struct fanotify_event_metadata *)event_buf;
-	verify_event(0, event, FAN_MODIFY, "");
+	verify_event(0, event, FAN_MODIFY, tc->report_name ? fname : "");
 	event = FAN_EVENT_NEXT(event, ret);
 	if (tc->nevents > 1) {
-		verify_event(0, event, FAN_CLOSE_NOWRITE, "");
+		verify_event(0, event, FAN_CLOSE_NOWRITE,
+			     tc->report_name ? (tc->ondir ? "." : tc->close_nowrite) : "");
 		event = FAN_EVENT_NEXT(event, ret);
 	}
 	if (ret > 0) {
@@ -304,8 +334,11 @@ static void test_fanotify(unsigned int n)
 
 static void setup(void)
 {
+	fan_report_dfid_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_DFID_NAME,
+									  MOUNT_PATH);
+
 	SAFE_MKDIR(MOUNT_NAME, 0755);
-	SAFE_MOUNT(MOUNT_NAME, MOUNT_NAME, "none", MS_BIND, NULL);
+	SAFE_MOUNT(MOUNT_PATH, MOUNT_NAME, "none", MS_BIND, NULL);
 	mount_created = 1;
 	SAFE_CHDIR(MOUNT_NAME);
 	SAFE_MKDIR(DIR_NAME, 0755);
@@ -330,6 +363,8 @@ static struct tst_test test = {
 	.tcnt = ARRAY_SIZE(tcases),
 	.setup = setup,
 	.cleanup = cleanup,
+	.mount_device = 1,
+	.mntpoint = MOUNT_PATH,
 	.needs_tmpdir = 1,
 	.needs_root = 1,
 	.tags = (const struct tst_tag[]) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 1/5] syscalls/fanotify: Generalize check for FAN_REPORT_FID support
  2020-12-04  9:59 ` [LTP] [PATCH 1/5] syscalls/fanotify: Generalize check for FAN_REPORT_FID support Amir Goldstein
@ 2020-12-04 13:52   ` Petr Vorel
  2020-12-07 10:48   ` Jan Kara
  1 sibling, 0 replies; 23+ messages in thread
From: Petr Vorel @ 2020-12-04 13:52 UTC (permalink / raw)
  To: ltp

Hi Amir,

> Generalize the helpers to be able to check any fanotify_init() flags
> and pass FAN_REPORT_FID as argument in call sites.

> Add helper fanotify_init_flags_supported_by_kernel() to check for
> kernel support for fanotify_init flags without checking fs support
> for those flags.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 2/5] syscalls/fanotify: Use generic checks for fanotify_init flags
  2020-12-04  9:59 ` [LTP] [PATCH 2/5] syscalls/fanotify: Use generic checks for fanotify_init flags Amir Goldstein
@ 2020-12-04 13:55   ` Petr Vorel
  2020-12-07 10:52   ` Jan Kara
  1 sibling, 0 replies; 23+ messages in thread
From: Petr Vorel @ 2020-12-04 13:55 UTC (permalink / raw)
  To: ltp

Hi Amir,

> Convert remaining tests to SAFE_FANOTIFY_INIT and use the generic
> helpers to check requires kernel/fs support for fanotify_init flags
> in advance.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 3/5] syscalls/fanotify09: Read variable length events
  2020-12-04  9:59 ` [LTP] [PATCH 3/5] syscalls/fanotify09: Read variable length events Amir Goldstein
@ 2020-12-04 14:16   ` Petr Vorel
  2020-12-07 10:55   ` Jan Kara
  2020-12-07 11:44   ` Petr Vorel
  2 siblings, 0 replies; 23+ messages in thread
From: Petr Vorel @ 2020-12-04 14:16 UTC (permalink / raw)
  To: ltp

Hi Amir,

> In preparation of testing events with filename info, teach the
> how to read variable length events and parse the name info.

Reviewed-by: Petr Vorel <pvorel@suse.cz>
LGTM.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 4/5] syscalls/fanotify09: Add test case with two non-dir children
  2020-12-04  9:59 ` [LTP] [PATCH 4/5] syscalls/fanotify09: Add test case with two non-dir children Amir Goldstein
@ 2020-12-04 14:19   ` Petr Vorel
  2020-12-07 11:01   ` Jan Kara
  1 sibling, 0 replies; 23+ messages in thread
From: Petr Vorel @ 2020-12-04 14:19 UTC (permalink / raw)
  To: ltp

Hi Amir,

> The existing test cases generate FAN_MODIFY event on a non-dir child
> and FAN_CLOSE_NOWRITE event the parent or sibling subdir.

> Add a test case that generates FAN_CLOSE_NOWRITE on a sibling non-dir
> child.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 5/5] syscalls/fanotify09: Add test case for events with filename info
  2020-12-04  9:59 ` [LTP] [PATCH 5/5] syscalls/fanotify09: Add test case for events with filename info Amir Goldstein
@ 2020-12-04 14:22   ` Petr Vorel
  2020-12-07 11:11   ` Jan Kara
  1 sibling, 0 replies; 23+ messages in thread
From: Petr Vorel @ 2020-12-04 14:22 UTC (permalink / raw)
  To: ltp

Hi Amir,

> Kernel v5.9 has a bug when there is a mark on filesystem or mount
> interested in events with filename AND there is an additional
> mark on a parent watching children.

> In some situations the event is reported to filesystem or mount mark
> without the filename info.

> This bug is fixed by kernel commit 7372e79c9eb9:
>   fanotify: fix logic of reporting name info with watched parent

You forget to add {"linux-git", "7372e79c9eb9"}, but we add it
before merge.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 0/5] Fanotify cleanup and test for v5.9 regression
  2020-12-04  9:59 [LTP] [PATCH 0/5] Fanotify cleanup and test for v5.9 regression Amir Goldstein
                   ` (4 preceding siblings ...)
  2020-12-04  9:59 ` [LTP] [PATCH 5/5] syscalls/fanotify09: Add test case for events with filename info Amir Goldstein
@ 2020-12-04 14:27 ` Petr Vorel
  5 siblings, 0 replies; 23+ messages in thread
From: Petr Vorel @ 2020-12-04 14:27 UTC (permalink / raw)
  To: ltp

Hi Amir,

> Hi Petr,

> Here are my followup patches to your great cleanup and
> a new test for a bug fix that is already included in v5.9.y.

> Note that I resurrected the helper from your V5 patches that
> you turned into a macro following Cyril's comment.
> I hope you both find the result satisfying.
Nice clean patchset, as usually. Thank you.
I'll do some testing on Monday.
@Jan, if you have time please have a quick look into the patchset.

> There are no direct calls to fanotify_init() after those
> cleanups (except for the intended use in fanotify14).
+1

> FYI, I have another test (inotify) for another v5.9 regression.
> The fix is queued for upstream, but did reach upstream yet, so
> I will post that test later.
+1

> Thanks,
> Amir.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 1/5] syscalls/fanotify: Generalize check for FAN_REPORT_FID support
  2020-12-04  9:59 ` [LTP] [PATCH 1/5] syscalls/fanotify: Generalize check for FAN_REPORT_FID support Amir Goldstein
  2020-12-04 13:52   ` Petr Vorel
@ 2020-12-07 10:48   ` Jan Kara
  1 sibling, 0 replies; 23+ messages in thread
From: Jan Kara @ 2020-12-07 10:48 UTC (permalink / raw)
  To: ltp

On Fri 04-12-20 11:59:26, Amir Goldstein wrote:
> Generalize the helpers to be able to check any fanotify_init() flags
> and pass FAN_REPORT_FID as argument in call sites.
> 
> Add helper fanotify_init_flags_supported_by_kernel() to check for
> kernel support for fanotify_init flags without checking fs support
> for those flags.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  testcases/kernel/syscalls/fanotify/fanotify.h | 46 ++++++++++++-------
>  .../kernel/syscalls/fanotify/fanotify01.c     |  4 +-
>  .../kernel/syscalls/fanotify/fanotify13.c     |  2 +-
>  .../kernel/syscalls/fanotify/fanotify15.c     |  2 +-
>  .../kernel/syscalls/fanotify/fanotify16.c     |  2 +-
>  5 files changed, 34 insertions(+), 22 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index 82e03db26..8907db052 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -286,15 +286,15 @@ static inline int fanotify_events_supported_by_kernel(uint64_t mask)
>  
>  /*
>   * @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
> + * @return -1: @flags not supported in kernel
> + * @return -2: @flags not supported on tested filesystem (tested if @fname is not NULL)
>   */
> -static inline int fanotify_fan_report_fid_supported_on_fs(const char *fname)
> +static inline int fanotify_init_flags_supported_on_fs(unsigned int flags, const char *fname)
>  {
>  	int fd;
>  	int rval = 0;
>  
> -	fd = fanotify_init(FAN_CLASS_NOTIF | FAN_REPORT_FID, O_RDONLY);
> +	fd = fanotify_init(flags, O_RDONLY);
>  
>  	if (fd < 0) {
>  		if (errno == ENOSYS)
> @@ -306,7 +306,7 @@ static inline int fanotify_fan_report_fid_supported_on_fs(const char *fname)
>  		tst_brk(TBROK | TERRNO, "fanotify_init() failed");
>  	}
>  
> -	if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS, AT_FDCWD, fname) < 0) {
> +	if (fname && fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS, AT_FDCWD, fname) < 0) {
>  		if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV) {
>  			rval = -2;
>  		} else {
> @@ -321,20 +321,32 @@ static inline int fanotify_fan_report_fid_supported_on_fs(const char *fname)
>  	return rval;
>  }
>  
> -#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, "FAN_REPORT_FID not supported on %s filesystem", \
> -			tst_device->fs_type); \
> -	} while (0)
> +static inline int fanotify_init_flags_supported_by_kernel(unsigned int flags)
> +{
> +	return fanotify_init_flags_supported_on_fs(flags, NULL);
> +}
> +
> +typedef void (*tst_res_func_t)(const char *file, const int lineno,
> +			       int ttype, const char *fmt, ...);
> +
> +static inline void fanotify_init_flags_err_msg(const char *flags_str,
> +	const char *file, const int lineno, tst_res_func_t res_func, int fail)
> +{
> +	if (fail == -1)
> +		res_func(file, lineno, TCONF,
> +			 "%s not supported in kernel?", flags_str);
> +	if (fail == -2)
> +		res_func(file, lineno, TCONF,
> +			 "%s not supported on %s filesystem",
> +			 flags_str, tst_device->fs_type);
> +}
>  
> -#define FANOTIFY_FAN_REPORT_FID_ERR_MSG(fail) \
> -	FANOTIFY_FAN_REPORT_FID_ERR_MSG_(tst_res, (fail))
> +#define FANOTIFY_INIT_FLAGS_ERR_MSG(flags, fail) \
> +	fanotify_init_flags_err_msg(#flags, __FILE__, __LINE__, tst_res_, (fail))
>  
> -#define REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(fname) do { \
> -	FANOTIFY_FAN_REPORT_FID_ERR_MSG_(tst_brk, \
> -		fanotify_fan_report_fid_supported_on_fs(fname)); \
> +#define REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(flags, fname) do { \
> +	fanotify_init_flags_err_msg(#flags, __FILE__, __LINE__, tst_brk_, \
> +		fanotify_init_flags_supported_on_fs(flags, fname)); \
>  	} while (0)
>  
>  static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
> index d6d72dad9..cdb01730f 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify01.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
> @@ -90,7 +90,7 @@ static void test_fanotify(unsigned int n)
>  	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
>  
>  	if (fan_report_fid_unsupported && (tc->init_flags & FAN_REPORT_FID)) {
> -		FANOTIFY_FAN_REPORT_FID_ERR_MSG(fan_report_fid_unsupported);
> +		FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_FID, fan_report_fid_unsupported);
>  		return;
>  	}
>  
> @@ -334,7 +334,7 @@ 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);
> +	fan_report_fid_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_FID, fname);
>  	filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
>  }
>  
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
> index a402cdb13..c9cf10555 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify13.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
> @@ -256,7 +256,7 @@ out:
>  
>  static void do_setup(void)
>  {
> -	REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(MOUNT_PATH);
> +	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, MOUNT_PATH);
>  
>  	filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
>  
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify15.c b/testcases/kernel/syscalls/fanotify/fanotify15.c
> index 9e3748bc2..ba8259c7c 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify15.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify15.c
> @@ -270,7 +270,7 @@ static void do_test(unsigned int number)
>  static void do_setup(void)
>  {
>  	SAFE_MKDIR(TEST_DIR, 0755);
> -	REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(TEST_DIR);
> +	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, TEST_DIR);
>  	fanotify_fd = SAFE_FANOTIFY_INIT(FAN_REPORT_FID, O_RDONLY);
>  }
>  
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
> index a554c7d39..a4409df14 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify16.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify16.c
> @@ -551,7 +551,7 @@ check_match:
>  
>  static void setup(void)
>  {
> -	REQUIRE_FANOTIFY_FAN_REPORT_FID_SUPPORTED_ON_FS(MOUNT_PATH);
> +	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, MOUNT_PATH);
>  
>  	sprintf(dname1, "%s/%s", MOUNT_PATH, DIR_NAME1);
>  	sprintf(dname2, "%s/%s", MOUNT_PATH, DIR_NAME2);
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 2/5] syscalls/fanotify: Use generic checks for fanotify_init flags
  2020-12-04  9:59 ` [LTP] [PATCH 2/5] syscalls/fanotify: Use generic checks for fanotify_init flags Amir Goldstein
  2020-12-04 13:55   ` Petr Vorel
@ 2020-12-07 10:52   ` Jan Kara
  1 sibling, 0 replies; 23+ messages in thread
From: Jan Kara @ 2020-12-07 10:52 UTC (permalink / raw)
  To: ltp

On Fri 04-12-20 11:59:27, Amir Goldstein wrote:
> Convert remaining tests to SAFE_FANOTIFY_INIT and use the generic
> helpers to check requires kernel/fs support for fanotify_init flags
> in advance.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  .../kernel/syscalls/fanotify/fanotify10.c     | 26 ++++++++-----------
>  .../kernel/syscalls/fanotify/fanotify11.c     | 21 ++++++---------
>  .../kernel/syscalls/fanotify/fanotify16.c     | 14 ++--------
>  3 files changed, 21 insertions(+), 40 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> index 404e57daa..cc164359f 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> @@ -57,6 +57,7 @@ static unsigned int fanotify_class[] = {
>  	FAN_REPORT_DFID_NAME_FID,
>  };
>  #define NUM_CLASSES ARRAY_SIZE(fanotify_class)
> +#define NUM_PRIORITIES 3
>  
>  #define GROUPS_PER_PRIO 3
>  
> @@ -64,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 fan_report_dfid_unsupported;
>  static int filesystem_mark_unsupported;
>  
>  #define MOUNT_PATH "fs_mnt"
> @@ -294,21 +296,8 @@ static int create_fanotify_groups(unsigned int n)
>  
>  	for (p = 0; p < num_classes; p++) {
>  		for (i = 0; i < GROUPS_PER_PRIO; i++) {
> -			fd_notify[p][i] = fanotify_init(fanotify_class[p] |
> -							FAN_NONBLOCK, O_RDONLY);
> -			if (fd_notify[p][i] == -1) {
> -				if (errno == EINVAL &&
> -				    fanotify_class[p] & FAN_REPORT_NAME) {
> -					tst_res(TCONF,
> -						"FAN_REPORT_NAME not supported by kernel?");
> -					/* Do not try creating this group again */
> -					num_classes--;
> -					return -1;
> -				}
> -
> -				tst_brk(TBROK | TERRNO,
> -					"fanotify_init(%x, 0) failed", fanotify_class[p]);
> -			}
> +			fd_notify[p][i] = SAFE_FANOTIFY_INIT(fanotify_class[p] |
> +							     FAN_NONBLOCK, O_RDONLY);
>  
>  			/*
>  			 * Add mark for each group.
> @@ -518,6 +507,13 @@ 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);
> +	fan_report_dfid_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_DFID_NAME,
> +									  MOUNT_PATH);
> +	if (fan_report_dfid_unsupported) {
> +		FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_DFID_NAME, fan_report_dfid_unsupported);
> +		/* Limit tests to legacy priority classes */
> +		num_classes = NUM_PRIORITIES;
> +	}
>  
>  	/* Create another bind mount at another path for generating events */
>  	SAFE_MKDIR(MNT2_PATH, 0755);
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify11.c b/testcases/kernel/syscalls/fanotify/fanotify11.c
> index 785b5c5a5..f3b60cecb 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify11.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify11.c
> @@ -36,6 +36,8 @@
>  #define gettid() syscall(SYS_gettid)
>  static int tid;
>  
> +static int fan_report_tid_unsupported;
> +
>  void *thread_create_file(void *arg LTP_ATTRIBUTE_UNUSED)
>  {
>  	char tid_file[64] = {0};
> @@ -63,17 +65,13 @@ void test01(unsigned int i)
>  			i, (tcases[i] & FAN_REPORT_TID) ? "with" : "without",
>  			tgid, tid, event.pid);
>  
> -	fd_notify = fanotify_init(tcases[i], 0);
> -	if (fd_notify < 0) {
> -		if (errno == EINVAL && (tcases[i] & FAN_REPORT_TID)) {
> -			tst_res(TCONF,
> -				"FAN_REPORT_TID not supported in kernel?");
> -			return;
> -		}
> -		tst_brk(TBROK | TERRNO, "fanotify_init(0x%x, 0) failed",
> -				tcases[i]);
> +	if (fan_report_tid_unsupported && (tcases[i] & FAN_REPORT_TID)) {
> +		FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_TID, fan_report_tid_unsupported);
> +		return;
>  	}
>  
> +	fd_notify = SAFE_FANOTIFY_INIT(tcases[i], 0);
> +
>  	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD,
>  			FAN_ALL_EVENTS | FAN_EVENT_ON_CHILD, AT_FDCWD, ".");
>  
> @@ -96,10 +94,7 @@ void test01(unsigned int i)
>  
>  static void setup(void)
>  {
> -	int fd;
> -
> -	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
> -	SAFE_CLOSE(fd);
> +	fan_report_tid_unsupported = fanotify_init_flags_supported_by_kernel(FAN_REPORT_TID);
>  }
>  
>  static struct tst_test test = {
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
> index a4409df14..5ffaec92f 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify16.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify16.c
> @@ -158,17 +158,7 @@ static void do_test(unsigned int number)
>  
>  	tst_res(TINFO, "Test #%d: %s", number, tc->tname);
>  
> -	fd_notify = fanotify_init(group->flag, 0);
> -	if (fd_notify == -1) {
> -		if (errno == EINVAL) {
> -			tst_res(TCONF,
> -				"%s not supported by kernel", group->name);
> -			return;
> -		}
> -
> -		tst_brk(TBROK | TERRNO,
> -			"fanotify_init(%s, 0) failed", group->name);
> -	}
> +	fd_notify = SAFE_FANOTIFY_INIT(group->flag, 0);
>  
>  	/*
>  	 * Watch dir modify events with name in filesystem/dir
> @@ -551,7 +541,7 @@ check_match:
>  
>  static void setup(void)
>  {
> -	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, MOUNT_PATH);
> +	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_DIR_FID, MOUNT_PATH);
>  
>  	sprintf(dname1, "%s/%s", MOUNT_PATH, DIR_NAME1);
>  	sprintf(dname2, "%s/%s", MOUNT_PATH, DIR_NAME2);
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 3/5] syscalls/fanotify09: Read variable length events
  2020-12-04  9:59 ` [LTP] [PATCH 3/5] syscalls/fanotify09: Read variable length events Amir Goldstein
  2020-12-04 14:16   ` Petr Vorel
@ 2020-12-07 10:55   ` Jan Kara
  2020-12-07 11:44   ` Petr Vorel
  2 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2020-12-07 10:55 UTC (permalink / raw)
  To: ltp

On Fri 04-12-20 11:59:28, Amir Goldstein wrote:
> In preparation of testing events with filename info, teach the
> how to read variable length events and parse the name info.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  .../kernel/syscalls/fanotify/fanotify09.c     | 88 +++++++++++++------
>  1 file changed, 60 insertions(+), 28 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
> index daeb712d2..7bb901cf3 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> @@ -138,42 +138,73 @@ static void cleanup_fanotify_groups(void)
>  }
>  
>  static void event_res(int ttype, int group,
> -		      struct fanotify_event_metadata *event)
> +		      struct fanotify_event_metadata *event,
> +		      const char *filename)
>  {
> -	int len;
> -	sprintf(symlnk, "/proc/self/fd/%d", event->fd);
> -	len = readlink(symlnk, fdpath, sizeof(fdpath));
> -	if (len < 0)
> -		len = 0;
> -	fdpath[len] = 0;
> -	tst_res(ttype, "group %d got event: mask %llx pid=%u fd=%d path=%s",
> +	if (event->fd != FAN_NOFD) {
> +		int len = 0;
> +		sprintf(symlnk, "/proc/self/fd/%d", event->fd);
> +		len = readlink(symlnk, fdpath, sizeof(fdpath));
> +		if (len < 0)
> +			len = 0;
> +		fdpath[len] = 0;
> +		filename = fdpath;
> +	}
> +	tst_res(ttype, "group %d got event: mask %llx pid=%u fd=%d filename=%s",
>  		group, (unsigned long long)event->mask,
> -		(unsigned)event->pid, event->fd, fdpath);
> +		(unsigned)event->pid, event->fd, filename);
> +}
> +
> +static const char *event_filename(struct fanotify_event_metadata *event)
> +{
> +	struct fanotify_event_info_fid *event_fid;
> +	struct file_handle *file_handle;
> +	const char *filename, *end;
> +
> +	if (event->event_len <= FAN_EVENT_METADATA_LEN)
> +		return "";
> +
> +	event_fid = (struct fanotify_event_info_fid *)(event + 1);
> +	file_handle = (struct file_handle *)event_fid->handle;
> +	filename = (char *)file_handle->f_handle + file_handle->handle_bytes;
> +	end = (char *)event_fid + event_fid->hdr.len;
> +
> +	/* End of event_fid could have name, zero padding, both or none */
> +	return (filename == end) ? "" : filename;
>  }
>  
>  static void verify_event(int group, struct fanotify_event_metadata *event,
> -			 uint32_t expect)
> +			 uint32_t expect, const char *expect_filename)
>  {
> +	const char *filename = event_filename(event);
> +
>  	if (event->mask != expect) {
>  		tst_res(TFAIL, "group %d got event: mask %llx (expected %llx) "
> -			"pid=%u fd=%d", group, (unsigned long long)event->mask,
> +			"pid=%u fd=%d filename=%s", group, (unsigned long long)event->mask,
>  			(unsigned long long)expect,
> -			(unsigned)event->pid, event->fd);
> +			(unsigned)event->pid, event->fd, filename);
>  	} else if (event->pid != getpid()) {
>  		tst_res(TFAIL, "group %d got event: mask %llx pid=%u "
> -			"(expected %u) fd=%d", group,
> +			"(expected %u) fd=%d filename=%s", group,
>  			(unsigned long long)event->mask, (unsigned)event->pid,
> -			(unsigned)getpid(), event->fd);
> +			(unsigned)getpid(), event->fd, filename);
> +	} else if (strcmp(filename, expect_filename)) {
> +		tst_res(TFAIL, "group %d got event: mask %llx pid=%u "
> +			"fd=%d filename='%s' (expected '%s')", group,
> +			(unsigned long long)event->mask, (unsigned)event->pid,
> +			event->fd, filename, expect_filename);
>  	} else {
> -		event_res(TPASS, group, event);
> +		event_res(TPASS, group, event, filename);
>  	}
> +	if (event->fd != FAN_NOFD)
> +		SAFE_CLOSE(event->fd);
>  }
>  
>  static void test_fanotify(unsigned int n)
>  {
>  	int ret, dirfd;
>  	unsigned int i;
> -	struct fanotify_event_metadata *event, *ev;
> +	struct fanotify_event_metadata *event;
>  	struct tcase *tc = &tcases[n];
>  
>  	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
> @@ -210,20 +241,21 @@ static void test_fanotify(unsigned int n)
>  			ret, tc->nevents * (int)FAN_EVENT_METADATA_LEN);
>  	}
>  	event = (struct fanotify_event_metadata *)event_buf;
> -	verify_event(0, event, FAN_MODIFY);
> -	if (tc->ondir)
> -		verify_event(0, event + 1, FAN_CLOSE_NOWRITE);
> -	if (ret > tc->nevents * (int)FAN_EVENT_METADATA_LEN) {
> +	verify_event(0, event, FAN_MODIFY, "");
> +	event = FAN_EVENT_NEXT(event, ret);
> +	if (tc->ondir) {
> +		verify_event(0, event, FAN_CLOSE_NOWRITE, "");
> +		event = FAN_EVENT_NEXT(event, ret);
> +	}
> +	if (ret > 0) {
>  		tst_res(TFAIL,
> -			"first group got more than %d events (%d > %d)",
> -			tc->nevents, ret,
> -			tc->nevents * (int)FAN_EVENT_METADATA_LEN);
> +			"first group got more than %d events (%d bytes)",
> +			tc->nevents, ret);
>  	}
>  	/* Close all file descriptors of read events */
> -	for (ev = event; ret >= (int)FAN_EVENT_METADATA_LEN; ev++) {
> -		if (ev->fd != FAN_NOFD)
> -			SAFE_CLOSE(ev->fd);
> -		ret -= (int)FAN_EVENT_METADATA_LEN;
> +	for (; FAN_EVENT_OK(event, ret); FAN_EVENT_NEXT(event, ret)) {
> +		if (event->fd != FAN_NOFD)
> +			SAFE_CLOSE(event->fd);
>  	}
>  
>  	/*
> @@ -233,7 +265,7 @@ static void test_fanotify(unsigned int n)
>  	for (i = 1; i < NUM_GROUPS; i++) {
>  		ret = read(fd_notify[i], event_buf, FAN_EVENT_METADATA_LEN);
>  		if (ret > 0) {
> -			event_res(TFAIL, i, event);
> +			event_res(TFAIL, i, event, "");
>  			if (event->fd != FAN_NOFD)
>  				SAFE_CLOSE(event->fd);
>  			continue;
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 4/5] syscalls/fanotify09: Add test case with two non-dir children
  2020-12-04  9:59 ` [LTP] [PATCH 4/5] syscalls/fanotify09: Add test case with two non-dir children Amir Goldstein
  2020-12-04 14:19   ` Petr Vorel
@ 2020-12-07 11:01   ` Jan Kara
  1 sibling, 0 replies; 23+ messages in thread
From: Jan Kara @ 2020-12-07 11:01 UTC (permalink / raw)
  To: ltp

On Fri 04-12-20 11:59:29, Amir Goldstein wrote:
> The existing test cases generate FAN_MODIFY event on a non-dir child
> and FAN_CLOSE_NOWRITE event the parent or sibling subdir.
> 
> Add a test case that generates FAN_CLOSE_NOWRITE on a sibling non-dir
> child.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  .../kernel/syscalls/fanotify/fanotify09.c     | 38 ++++++++++++++-----
>  1 file changed, 28 insertions(+), 10 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
> index 7bb901cf3..0f1923981 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> @@ -55,13 +55,14 @@ static char event_buf[EVENT_BUF_LEN];
>  
>  #define MOUNT_NAME "mntpoint"
>  #define DIR_NAME "testdir"
> +#define FILE2_NAME "testfile"
>  static int mount_created;
>  
>  static struct tcase {
>  	const char *tname;
>  	struct fanotify_mark_type mark;
>  	unsigned int ondir;
> -	const char *testdir;
> +	const char *close_nowrite;
>  	int nevents;
>  } tcases[] = {
>  	{
> @@ -92,6 +93,13 @@ static struct tcase {
>  		DIR_NAME,
>  		2,
>  	},
> +	{
> +		"Events on non-dir children with both parent and mount marks",
> +		INIT_FANOTIFY_MARK_TYPE(MOUNT),
> +		0,
> +		FILE2_NAME,
> +		2,
> +	},
>  };
>  
>  static void create_fanotify_groups(struct tcase *tc)
> @@ -112,7 +120,7 @@ static void create_fanotify_groups(struct tcase *tc)
>  		SAFE_FANOTIFY_MARK(fd_notify[i],
>  				    FAN_MARK_ADD | mark->flag,
>  				    FAN_CLOSE_NOWRITE | onchild,
> -				    AT_FDCWD, tc->testdir);
> +				    AT_FDCWD, tc->close_nowrite);
>  
>  		/*
>  		 * Add inode mark on parent for each group with MODIFY event,
> @@ -216,9 +224,9 @@ static void test_fanotify(unsigned int n)
>  	 */
>  	SAFE_FILE_PRINTF(fname, "1");
>  	/*
> -	 * generate FAN_CLOSE_NOWRITE event on a testdir (subdir or ".")
> +	 * generate FAN_CLOSE_NOWRITE event on a child, subdir or "."
>  	 */
> -	dirfd = SAFE_OPEN(tc->testdir, O_RDONLY);
> +	dirfd = SAFE_OPEN(tc->close_nowrite, O_RDONLY);
>  	if (dirfd >= 0)
>  		SAFE_CLOSE(dirfd);
>  
> @@ -243,7 +251,7 @@ static void test_fanotify(unsigned int n)
>  	event = (struct fanotify_event_metadata *)event_buf;
>  	verify_event(0, event, FAN_MODIFY, "");
>  	event = FAN_EVENT_NEXT(event, ret);
> -	if (tc->ondir) {
> +	if (tc->nevents > 1) {
>  		verify_event(0, event, FAN_CLOSE_NOWRITE, "");
>  		event = FAN_EVENT_NEXT(event, ret);
>  	}
> @@ -260,14 +268,23 @@ static void test_fanotify(unsigned int n)
>  
>  	/*
>  	 * Then verify the rest of the groups did not get the MODIFY event and
> -	 * did not get the FAN_CLOSE_NOWRITE event on testdir.
> +	 * got the FAN_CLOSE_NOWRITE event only on a non-directory.
>  	 */
>  	for (i = 1; i < NUM_GROUPS; i++) {
> -		ret = read(fd_notify[i], event_buf, FAN_EVENT_METADATA_LEN);
> +		ret = read(fd_notify[i], event_buf, EVENT_BUF_LEN);
>  		if (ret > 0) {
> -			event_res(TFAIL, i, event, "");
> -			if (event->fd != FAN_NOFD)
> -				SAFE_CLOSE(event->fd);
> +			uint32_t expect = 0;
> +
> +			if (tc->nevents > 1 && !tc->ondir)
> +				expect = FAN_CLOSE_NOWRITE;
> +
> +			event = (struct fanotify_event_metadata *)event_buf;
> +			verify_event(i, event, expect, "");
> +
> +			for (; FAN_EVENT_OK(event, ret); FAN_EVENT_NEXT(event, ret)) {
> +				if (event->fd != FAN_NOFD)
> +					SAFE_CLOSE(event->fd);
> +			}
>  			continue;
>  		}
>  
> @@ -295,6 +312,7 @@ static void setup(void)
>  
>  	sprintf(fname, "tfile_%d", getpid());
>  	SAFE_FILE_PRINTF(fname, "1");
> +	SAFE_FILE_PRINTF(FILE2_NAME, "1");
>  }
>  
>  static void cleanup(void)
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 5/5] syscalls/fanotify09: Add test case for events with filename info
  2020-12-04  9:59 ` [LTP] [PATCH 5/5] syscalls/fanotify09: Add test case for events with filename info Amir Goldstein
  2020-12-04 14:22   ` Petr Vorel
@ 2020-12-07 11:11   ` Jan Kara
  1 sibling, 0 replies; 23+ messages in thread
From: Jan Kara @ 2020-12-07 11:11 UTC (permalink / raw)
  To: ltp

On Fri 04-12-20 11:59:30, Amir Goldstein wrote:
> Kernel v5.9 has a bug when there is a mark on filesystem or mount
> interested in events with filename AND there is an additional
> mark on a parent watching children.
> 
> In some situations the event is reported to filesystem or mount mark
> without the filename info.
> 
> This bug is fixed by kernel commit 7372e79c9eb9:
>   fanotify: fix logic of reporting name info with watched parent
> 
> The test case requires a blockdev filesystem.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  .../kernel/syscalls/fanotify/fanotify09.c     | 49 ++++++++++++++++---
>  1 file changed, 42 insertions(+), 7 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
> index 0f1923981..25b6b8be1 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> @@ -19,6 +19,10 @@
>   * Test case #2 is a regression test for commit 55bf882c7f13:
>   *
>   *      fanotify: fix merging marks masks with FAN_ONDIR
> + *
> + * Test case #5 is a regression test for commit 7372e79c9eb9:
> + *
> + *      fanotify: fix logic of reporting name info with watched parent
>   */
>  #define _GNU_SOURCE
>  #include "config.h"
> @@ -53,15 +57,19 @@ static int fd_notify[NUM_GROUPS];
>  
>  static char event_buf[EVENT_BUF_LEN];
>  
> +#define MOUNT_PATH "fs_mnt"
>  #define MOUNT_NAME "mntpoint"
>  #define DIR_NAME "testdir"
>  #define FILE2_NAME "testfile"
>  static int mount_created;
>  
> +static int fan_report_dfid_unsupported;
> +
>  static struct tcase {
>  	const char *tname;
>  	struct fanotify_mark_type mark;
>  	unsigned int ondir;
> +	unsigned int report_name;
>  	const char *close_nowrite;
>  	int nevents;
>  } tcases[] = {
> @@ -69,6 +77,7 @@ static struct tcase {
>  		"Events on non-dir child with both parent and mount marks",
>  		INIT_FANOTIFY_MARK_TYPE(MOUNT),
>  		0,
> +		0,
>  		DIR_NAME,
>  		1,
>  	},
> @@ -76,6 +85,7 @@ static struct tcase {
>  		"Events on non-dir child and subdir with both parent and mount marks",
>  		INIT_FANOTIFY_MARK_TYPE(MOUNT),
>  		FAN_ONDIR,
> +		0,
>  		DIR_NAME,
>  		2,
>  	},
> @@ -83,6 +93,7 @@ static struct tcase {
>  		"Events on non-dir child and parent with both parent and mount marks",
>  		INIT_FANOTIFY_MARK_TYPE(MOUNT),
>  		FAN_ONDIR,
> +		0,
>  		".",
>  		2,
>  	},
> @@ -90,6 +101,7 @@ static struct tcase {
>  		"Events on non-dir child and subdir with both parent and subdir marks",
>  		INIT_FANOTIFY_MARK_TYPE(INODE),
>  		FAN_ONDIR,
> +		0,
>  		DIR_NAME,
>  		2,
>  	},
> @@ -97,6 +109,15 @@ static struct tcase {
>  		"Events on non-dir children with both parent and mount marks",
>  		INIT_FANOTIFY_MARK_TYPE(MOUNT),
>  		0,
> +		0,
> +		FILE2_NAME,
> +		2,
> +	},
> +	{
> +		"Events on non-dir child with both parent and mount marks and filename info",
> +		INIT_FANOTIFY_MARK_TYPE(MOUNT),
> +		0,
> +		FAN_REPORT_DFID_NAME,
>  		FILE2_NAME,
>  		2,
>  	},
> @@ -105,12 +126,15 @@ static struct tcase {
>  static void create_fanotify_groups(struct tcase *tc)
>  {
>  	struct fanotify_mark_type *mark = &tc->mark;
> -	unsigned int i, onchild, ondir = tc->ondir;
> +	unsigned int i, onchild, report_name, ondir = tc->ondir;
>  
>  	for (i = 0; i < NUM_GROUPS; i++) {
> -		fd_notify[i] = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF |
> -						  FAN_NONBLOCK,
> -						  O_RDONLY);
> +		/*
> +		 * The first group may request events with filename info.
> +		 */
> +		report_name = (i == 0) ? tc->report_name : 0;
> +		fd_notify[i] = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | report_name |
> +						  FAN_NONBLOCK, O_RDONLY);
>  
>  		/*
>  		 * Add subdir or mount mark for each group with CLOSE event,
> @@ -217,6 +241,11 @@ static void test_fanotify(unsigned int n)
>  
>  	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
>  
> +	if (fan_report_dfid_unsupported && tc->report_name) {
> +		FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_DFID_NAME, fan_report_dfid_unsupported);
> +		return;
> +	}
> +
>  	create_fanotify_groups(tc);
>  
>  	/*
> @@ -249,10 +278,11 @@ static void test_fanotify(unsigned int n)
>  			ret, tc->nevents * (int)FAN_EVENT_METADATA_LEN);
>  	}
>  	event = (struct fanotify_event_metadata *)event_buf;
> -	verify_event(0, event, FAN_MODIFY, "");
> +	verify_event(0, event, FAN_MODIFY, tc->report_name ? fname : "");
>  	event = FAN_EVENT_NEXT(event, ret);
>  	if (tc->nevents > 1) {
> -		verify_event(0, event, FAN_CLOSE_NOWRITE, "");
> +		verify_event(0, event, FAN_CLOSE_NOWRITE,
> +			     tc->report_name ? (tc->ondir ? "." : tc->close_nowrite) : "");
>  		event = FAN_EVENT_NEXT(event, ret);
>  	}
>  	if (ret > 0) {
> @@ -304,8 +334,11 @@ static void test_fanotify(unsigned int n)
>  
>  static void setup(void)
>  {
> +	fan_report_dfid_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_DFID_NAME,
> +									  MOUNT_PATH);
> +
>  	SAFE_MKDIR(MOUNT_NAME, 0755);
> -	SAFE_MOUNT(MOUNT_NAME, MOUNT_NAME, "none", MS_BIND, NULL);
> +	SAFE_MOUNT(MOUNT_PATH, MOUNT_NAME, "none", MS_BIND, NULL);
>  	mount_created = 1;
>  	SAFE_CHDIR(MOUNT_NAME);
>  	SAFE_MKDIR(DIR_NAME, 0755);
> @@ -330,6 +363,8 @@ static struct tst_test test = {
>  	.tcnt = ARRAY_SIZE(tcases),
>  	.setup = setup,
>  	.cleanup = cleanup,
> +	.mount_device = 1,
> +	.mntpoint = MOUNT_PATH,
>  	.needs_tmpdir = 1,
>  	.needs_root = 1,
>  	.tags = (const struct tst_tag[]) {
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 3/5] syscalls/fanotify09: Read variable length events
  2020-12-04  9:59 ` [LTP] [PATCH 3/5] syscalls/fanotify09: Read variable length events Amir Goldstein
  2020-12-04 14:16   ` Petr Vorel
  2020-12-07 10:55   ` Jan Kara
@ 2020-12-07 11:44   ` Petr Vorel
  2020-12-07 11:57     ` Petr Vorel
  2020-12-07 14:07     ` Amir Goldstein
  2 siblings, 2 replies; 23+ messages in thread
From: Petr Vorel @ 2020-12-07 11:44 UTC (permalink / raw)
  To: ltp

Hi Amir,

> In preparation of testing events with filename info, teach the
> how to read variable length events and parse the name info.

This commit broke umount() on old kernels. (LTP lib doing multiple attempts is
useless here). Not sure what exactly is being used, it's not by staing in
mounted directory. Any idea how to fix it?

Testing on SLES 4.4.21 (with many patches) on btrfs, I could also reproduce it
on tmpfs.

Kind regards,
Petr

# ./fanotify09
tst_test.c:1250: TINFO: Timeout per run is 0h 05m 00s
fanotify09.c:210: TINFO: Test #0: Events on non-dir child with both parent and mount marks
fanotify09.c:155: TPASS: group 0 got event: mask 2 pid=10770 fd=6 filename=/tmp/RLmVI1/mntpoint/tfile_10770
fanotify09.c:155: TFAIL: group 1 got event: mask 0 pid=0 fd=0 filename=/dev/pts/0
fanotify09.c:155: TFAIL: group 2 got event: mask 0 pid=0 fd=-1 filename=
fanotify09.c:210: TINFO: Test #1: Events on non-dir child and subdir with both parent and mount marks
fanotify09.c:155: TPASS: group 0 got event: mask 2 pid=10770 fd=7 filename=/tmp/RLmVI1/mntpoint/tfile_10770
fanotify09.c:155: TPASS: group 0 got event: mask 10 pid=10770 fd=8 filename=/tmp/RLmVI1/mntpoint/testdir
fanotify09.c:253: TFAIL: first group got more than 2 events (24 bytes)
fanotify09.c:155: TFAIL: group 1 got event: mask 10 pid=10770 fd=-1 filename=
fanotify09.c:155: TFAIL: group 2 got event: mask 10 pid=10770 fd=-1 filename=
fanotify09.c:210: TINFO: Test #2: Events on non-dir child and parent with both parent and mount marks
fanotify09.c:155: TPASS: group 0 got event: mask 2 pid=10770 fd=9 filename=/tmp/RLmVI1/mntpoint/tfile_10770
fanotify09.c:155: TPASS: group 0 got event: mask 10 pid=10770 fd=10 filename=/tmp/RLmVI1/mntpoint
fanotify09.c:155: TFAIL: group 1 got event: mask 10 pid=10770 fd=-1 filename=
fanotify09.c:155: TFAIL: group 2 got event: mask 10 pid=10770 fd=-1 filename=
fanotify09.c:210: TINFO: Test #3: Events on non-dir child and subdir with both parent and subdir marks
fanotify09.c:155: TPASS: group 0 got event: mask 2 pid=10770 fd=11 filename=/tmp/RLmVI1/mntpoint/tfile_10770
fanotify09.c:155: TPASS: group 0 got event: mask 10 pid=10770 fd=12 filename=/tmp/RLmVI1/mntpoint/testdir
fanotify09.c:283: TPASS: group 1 got no event
fanotify09.c:283: TPASS: group 2 got no event

pvorel: cwd before calling cleanup_fanotify_groups() is /tmp/mKrYUr/mntpoint
then in cleanup we cd .. (/tmp/mKrYUr)

tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try  1...
tst_device.c:394: TINFO: Likely gvfsd-trash is probing newly mounted fs, kill it to speed up tests.
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try  2...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try  3...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try  4...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try  5...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try  6...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try  7...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try  8...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try  9...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 10...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 11...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 12...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 13...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 14...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 15...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 16...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 17...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 18...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 19...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 20...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 21...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 22...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 23...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 24...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 25...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 26...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 27...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 28...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 29...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 30...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 31...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 32...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 33...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 34...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 35...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 36...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 37...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 38...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 39...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 40...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 41...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 42...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 43...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 44...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 45...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 46...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 47...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 48...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 49...
tst_device.c:390: TINFO: umount('mntpoint') failed with EBUSY, try 50...
tst_device.c:400: TWARN: Failed to umount('mntpoint') after 50 retries
fanotify09.c:307: TWARN: umount failed: EBUSY (16)

HINT: You _MAY_ be missing kernel fixes, see:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=54a307ba8d3c
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b469e7e47c8a
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=55bf882c7f13

Summary:
passed   9
failed   7
skipped  0
warnings 2
tst_tmpdir.c:337: TWARN: tst_rmdir: rmobj(/tmp/RLmVI1) failed: remove(/tmp/RLmVI1/mntpoint) failed; errno=16: EBUSY


> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  .../kernel/syscalls/fanotify/fanotify09.c     | 88 +++++++++++++------
>  1 file changed, 60 insertions(+), 28 deletions(-)

> diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
> index daeb712d2..7bb901cf3 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> @@ -138,42 +138,73 @@ static void cleanup_fanotify_groups(void)
>  }

>  static void event_res(int ttype, int group,
> -		      struct fanotify_event_metadata *event)
> +		      struct fanotify_event_metadata *event,
> +		      const char *filename)
>  {
> -	int len;
> -	sprintf(symlnk, "/proc/self/fd/%d", event->fd);
> -	len = readlink(symlnk, fdpath, sizeof(fdpath));
> -	if (len < 0)
> -		len = 0;
> -	fdpath[len] = 0;
> -	tst_res(ttype, "group %d got event: mask %llx pid=%u fd=%d path=%s",
> +	if (event->fd != FAN_NOFD) {
> +		int len = 0;
> +		sprintf(symlnk, "/proc/self/fd/%d", event->fd);
> +		len = readlink(symlnk, fdpath, sizeof(fdpath));
> +		if (len < 0)
> +			len = 0;
> +		fdpath[len] = 0;
> +		filename = fdpath;
> +	}
> +	tst_res(ttype, "group %d got event: mask %llx pid=%u fd=%d filename=%s",
>  		group, (unsigned long long)event->mask,
> -		(unsigned)event->pid, event->fd, fdpath);
> +		(unsigned)event->pid, event->fd, filename);
> +}
> +
> +static const char *event_filename(struct fanotify_event_metadata *event)
> +{
> +	struct fanotify_event_info_fid *event_fid;
> +	struct file_handle *file_handle;
> +	const char *filename, *end;
> +
> +	if (event->event_len <= FAN_EVENT_METADATA_LEN)
> +		return "";
> +
> +	event_fid = (struct fanotify_event_info_fid *)(event + 1);
> +	file_handle = (struct file_handle *)event_fid->handle;
> +	filename = (char *)file_handle->f_handle + file_handle->handle_bytes;
> +	end = (char *)event_fid + event_fid->hdr.len;
> +
> +	/* End of event_fid could have name, zero padding, both or none */
> +	return (filename == end) ? "" : filename;
>  }

>  static void verify_event(int group, struct fanotify_event_metadata *event,
> -			 uint32_t expect)
> +			 uint32_t expect, const char *expect_filename)
>  {
> +	const char *filename = event_filename(event);
> +
>  	if (event->mask != expect) {
>  		tst_res(TFAIL, "group %d got event: mask %llx (expected %llx) "
> -			"pid=%u fd=%d", group, (unsigned long long)event->mask,
> +			"pid=%u fd=%d filename=%s", group, (unsigned long long)event->mask,
>  			(unsigned long long)expect,
> -			(unsigned)event->pid, event->fd);
> +			(unsigned)event->pid, event->fd, filename);
>  	} else if (event->pid != getpid()) {
>  		tst_res(TFAIL, "group %d got event: mask %llx pid=%u "
> -			"(expected %u) fd=%d", group,
> +			"(expected %u) fd=%d filename=%s", group,
>  			(unsigned long long)event->mask, (unsigned)event->pid,
> -			(unsigned)getpid(), event->fd);
> +			(unsigned)getpid(), event->fd, filename);
> +	} else if (strcmp(filename, expect_filename)) {
> +		tst_res(TFAIL, "group %d got event: mask %llx pid=%u "
> +			"fd=%d filename='%s' (expected '%s')", group,
> +			(unsigned long long)event->mask, (unsigned)event->pid,
> +			event->fd, filename, expect_filename);
>  	} else {
> -		event_res(TPASS, group, event);
> +		event_res(TPASS, group, event, filename);
>  	}
> +	if (event->fd != FAN_NOFD)
> +		SAFE_CLOSE(event->fd);
>  }

>  static void test_fanotify(unsigned int n)
>  {
>  	int ret, dirfd;
>  	unsigned int i;
> -	struct fanotify_event_metadata *event, *ev;
> +	struct fanotify_event_metadata *event;
>  	struct tcase *tc = &tcases[n];

>  	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
> @@ -210,20 +241,21 @@ static void test_fanotify(unsigned int n)
>  			ret, tc->nevents * (int)FAN_EVENT_METADATA_LEN);
>  	}
>  	event = (struct fanotify_event_metadata *)event_buf;
> -	verify_event(0, event, FAN_MODIFY);
> -	if (tc->ondir)
> -		verify_event(0, event + 1, FAN_CLOSE_NOWRITE);
> -	if (ret > tc->nevents * (int)FAN_EVENT_METADATA_LEN) {
> +	verify_event(0, event, FAN_MODIFY, "");
> +	event = FAN_EVENT_NEXT(event, ret);
> +	if (tc->ondir) {
> +		verify_event(0, event, FAN_CLOSE_NOWRITE, "");
> +		event = FAN_EVENT_NEXT(event, ret);
> +	}
> +	if (ret > 0) {
>  		tst_res(TFAIL,
> -			"first group got more than %d events (%d > %d)",
> -			tc->nevents, ret,
> -			tc->nevents * (int)FAN_EVENT_METADATA_LEN);
> +			"first group got more than %d events (%d bytes)",
> +			tc->nevents, ret);
>  	}
>  	/* Close all file descriptors of read events */
> -	for (ev = event; ret >= (int)FAN_EVENT_METADATA_LEN; ev++) {
> -		if (ev->fd != FAN_NOFD)
> -			SAFE_CLOSE(ev->fd);
> -		ret -= (int)FAN_EVENT_METADATA_LEN;
> +	for (; FAN_EVENT_OK(event, ret); FAN_EVENT_NEXT(event, ret)) {
> +		if (event->fd != FAN_NOFD)
> +			SAFE_CLOSE(event->fd);
>  	}

>  	/*
> @@ -233,7 +265,7 @@ static void test_fanotify(unsigned int n)
>  	for (i = 1; i < NUM_GROUPS; i++) {
>  		ret = read(fd_notify[i], event_buf, FAN_EVENT_METADATA_LEN);
>  		if (ret > 0) {
> -			event_res(TFAIL, i, event);
> +			event_res(TFAIL, i, event, "");
>  			if (event->fd != FAN_NOFD)
>  				SAFE_CLOSE(event->fd);
>  			continue;

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 3/5] syscalls/fanotify09: Read variable length events
  2020-12-07 11:44   ` Petr Vorel
@ 2020-12-07 11:57     ` Petr Vorel
  2020-12-07 14:07     ` Amir Goldstein
  1 sibling, 0 replies; 23+ messages in thread
From: Petr Vorel @ 2020-12-07 11:57 UTC (permalink / raw)
  To: ltp

Hi Amir,

> > In preparation of testing events with filename info, teach the
> > how to read variable length events and parse the name info.

> This commit broke umount() on old kernels. (LTP lib doing multiple attempts is
> useless here). Not sure what exactly is being used, it's not by staing in
> mounted directory. Any idea how to fix it?

Using umount2(MOUNT_NAME, MNT_DETACH) obviously fixes umount, but not sure if
fixing something in the test would allow to use tst_umount().

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 3/5] syscalls/fanotify09: Read variable length events
  2020-12-07 11:44   ` Petr Vorel
  2020-12-07 11:57     ` Petr Vorel
@ 2020-12-07 14:07     ` Amir Goldstein
  2020-12-07 14:22       ` Petr Vorel
  1 sibling, 1 reply; 23+ messages in thread
From: Amir Goldstein @ 2020-12-07 14:07 UTC (permalink / raw)
  To: ltp

On Mon, Dec 7, 2020 at 1:44 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Amir,
>
> > In preparation of testing events with filename info, teach the
> > how to read variable length events and parse the name info.
>
> This commit broke umount() on old kernels. (LTP lib doing multiple attempts is
> useless here). Not sure what exactly is being used, it's not by staing in
> mounted directory. Any idea how to fix it?
>

--- a/testcases/kernel/syscalls/fanotify/fanotify09.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
@@ -265,6 +265,7 @@ static void test_fanotify(unsigned int n)
        for (i = 1; i < NUM_GROUPS; i++) {
                ret = read(fd_notify[i], event_buf, FAN_EVENT_METADATA_LEN);
                if (ret > 0) {
+                       event = (struct fanotify_event_metadata *)event_buf;
                        event_res(TFAIL, i, event, "");
                        if (event->fd != FAN_NOFD)
                                SAFE_CLOSE(event->fd);

The fix exists in the following patch, therefore I did not notice the
mid series regression.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 3/5] syscalls/fanotify09: Read variable length events
  2020-12-07 14:07     ` Amir Goldstein
@ 2020-12-07 14:22       ` Petr Vorel
  2020-12-07 16:17         ` Amir Goldstein
  0 siblings, 1 reply; 23+ messages in thread
From: Petr Vorel @ 2020-12-07 14:22 UTC (permalink / raw)
  To: ltp

Hi Amir,
> > > In preparation of testing events with filename info, teach the
> > > how to read variable length events and parse the name info.

> > This commit broke umount() on old kernels. (LTP lib doing multiple attempts is
> > useless here). Not sure what exactly is being used, it's not by staing in
> > mounted directory. Any idea how to fix it?


> --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> @@ -265,6 +265,7 @@ static void test_fanotify(unsigned int n)
>         for (i = 1; i < NUM_GROUPS; i++) {
>                 ret = read(fd_notify[i], event_buf, FAN_EVENT_METADATA_LEN);
>                 if (ret > 0) {
> +                       event = (struct fanotify_event_metadata *)event_buf;
>                         event_res(TFAIL, i, event, "");
>                         if (event->fd != FAN_NOFD)
>                                 SAFE_CLOSE(event->fd);

> The fix exists in the following patch, therefore I did not notice the
> mid series regression.
While this is valid to be added in this commit and I'll add it, it does not fix
our solution. I might not be clear: since this commit it's broken.
Thus any other tip?

Kind regards,
Petr

> Thanks,
> Amir.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 3/5] syscalls/fanotify09: Read variable length events
  2020-12-07 14:22       ` Petr Vorel
@ 2020-12-07 16:17         ` Amir Goldstein
  2020-12-08  7:30           ` Petr Vorel
  0 siblings, 1 reply; 23+ messages in thread
From: Amir Goldstein @ 2020-12-07 16:17 UTC (permalink / raw)
  To: ltp

On Mon, Dec 7, 2020 at 4:22 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Amir,
> > > > In preparation of testing events with filename info, teach the
> > > > how to read variable length events and parse the name info.
>
> > > This commit broke umount() on old kernels. (LTP lib doing multiple attempts is
> > > useless here). Not sure what exactly is being used, it's not by staing in
> > > mounted directory. Any idea how to fix it?
>
>
> > --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> > @@ -265,6 +265,7 @@ static void test_fanotify(unsigned int n)
> >         for (i = 1; i < NUM_GROUPS; i++) {
> >                 ret = read(fd_notify[i], event_buf, FAN_EVENT_METADATA_LEN);
> >                 if (ret > 0) {
> > +                       event = (struct fanotify_event_metadata *)event_buf;
> >                         event_res(TFAIL, i, event, "");
> >                         if (event->fd != FAN_NOFD)
> >                                 SAFE_CLOSE(event->fd);
>
> > The fix exists in the following patch, therefore I did not notice the
> > mid series regression.
> While this is valid to be added in this commit and I'll add it, it does not fix
> our solution. I might not be clear: since this commit it's broken.
> Thus any other tip?

So both a mid series regression and full series regression.
Lovely :)

Following patch needs this fix:

--- a/testcases/kernel/syscalls/fanotify/fanotify09.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
@@ -280,6 +280,7 @@ static void test_fanotify(unsigned int n)

                        event = (struct fanotify_event_metadata *)event_buf;
                        verify_event(i, event, expect, "");
+                       event = FAN_EVENT_NEXT(event, ret);

                        for (; FAN_EVENT_OK(event, ret);
FAN_EVENT_NEXT(event, ret)) {
                                if (event->fd != FAN_NOFD)

Pushed full fix series (including un-posted inotify test) to:
https://github.com/amir73il/ltp/commits/fsnotify-fixes

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [LTP] [PATCH 3/5] syscalls/fanotify09: Read variable length events
  2020-12-07 16:17         ` Amir Goldstein
@ 2020-12-08  7:30           ` Petr Vorel
  0 siblings, 0 replies; 23+ messages in thread
From: Petr Vorel @ 2020-12-08  7:30 UTC (permalink / raw)
  To: ltp

Hi Amir,

...
> > > The fix exists in the following patch, therefore I did not notice the
> > > mid series regression.
> > While this is valid to be added in this commit and I'll add it, it does not fix
> > our solution. I might not be clear: since this commit it's broken.
> > Thus any other tip?

> So both a mid series regression and full series regression.
> Lovely :)

> Following patch needs this fix:

> --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> @@ -280,6 +280,7 @@ static void test_fanotify(unsigned int n)

>                         event = (struct fanotify_event_metadata *)event_buf;
>                         verify_event(i, event, expect, "");
> +                       event = FAN_EVENT_NEXT(event, ret);

>                         for (; FAN_EVENT_OK(event, ret);
> FAN_EVENT_NEXT(event, ret)) {
>                                 if (event->fd != FAN_NOFD)

> Pushed full fix series (including un-posted inotify test) to:
> https://github.com/amir73il/ltp/commits/fsnotify-fixes

Thanks! I was wrong, next commit was broken due missing this,
as you had correctly fixed in your LTP fork.

Anyway, merged!

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2020-12-08  7:30 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-04  9:59 [LTP] [PATCH 0/5] Fanotify cleanup and test for v5.9 regression Amir Goldstein
2020-12-04  9:59 ` [LTP] [PATCH 1/5] syscalls/fanotify: Generalize check for FAN_REPORT_FID support Amir Goldstein
2020-12-04 13:52   ` Petr Vorel
2020-12-07 10:48   ` Jan Kara
2020-12-04  9:59 ` [LTP] [PATCH 2/5] syscalls/fanotify: Use generic checks for fanotify_init flags Amir Goldstein
2020-12-04 13:55   ` Petr Vorel
2020-12-07 10:52   ` Jan Kara
2020-12-04  9:59 ` [LTP] [PATCH 3/5] syscalls/fanotify09: Read variable length events Amir Goldstein
2020-12-04 14:16   ` Petr Vorel
2020-12-07 10:55   ` Jan Kara
2020-12-07 11:44   ` Petr Vorel
2020-12-07 11:57     ` Petr Vorel
2020-12-07 14:07     ` Amir Goldstein
2020-12-07 14:22       ` Petr Vorel
2020-12-07 16:17         ` Amir Goldstein
2020-12-08  7:30           ` Petr Vorel
2020-12-04  9:59 ` [LTP] [PATCH 4/5] syscalls/fanotify09: Add test case with two non-dir children Amir Goldstein
2020-12-04 14:19   ` Petr Vorel
2020-12-07 11:01   ` Jan Kara
2020-12-04  9:59 ` [LTP] [PATCH 5/5] syscalls/fanotify09: Add test case for events with filename info Amir Goldstein
2020-12-04 14:22   ` Petr Vorel
2020-12-07 11:11   ` Jan Kara
2020-12-04 14:27 ` [LTP] [PATCH 0/5] Fanotify cleanup and test for v5.9 regression Petr Vorel

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.