[LTP] [PATCH 0/4] More fanotify tests for v5.19

[LTP] [PATCH 0/4] More fanotify tests for v5.19

[Amir Goldstein]

Hi Petr,

I remembered I also have these 2 new test cases for a fix in v5.19-rc1.

The bug is quite niche and fix is not trivial to backport to LTS kernels,
so I chose not to run these test cases for kernel < 5.19, similar to the
approach we took with another fix in fanotify10.

Thanks,
Amir.

Amir Goldstein (4):
  syscalls/fanotify09: Cleanup open event fds on error
  syscalls/fanotify09: Verify if no events are expected
  syscalls/fanotify09: Tidy up the test to make it more readable
  syscalls/fanotify09: Add test cases for merge of ignore mask

 .../kernel/syscalls/fanotify/fanotify09.c     | 180 +++++++++++++-----
 1 file changed, 130 insertions(+), 50 deletions(-)

-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/4] syscalls/fanotify09: Cleanup open event fds on error

[Amir Goldstein]

Avoid breaking out of test, without closing all fds of events in buffer
to avoid failure to unmount fs on cleanup.

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

diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
index fea374689..60ffcb81b 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify09.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
@@ -238,6 +238,15 @@ static void verify_event(int group, struct fanotify_event_metadata *event,
 		SAFE_CLOSE(event->fd);
 }
 
+static void close_event_fds(struct fanotify_event_metadata *event, int buflen)
+{
+	/* Close all file descriptors of read events */
+	for (; FAN_EVENT_OK(event, buflen); FAN_EVENT_NEXT(event, buflen)) {
+		if (event->fd != FAN_NOFD)
+			SAFE_CLOSE(event->fd);
+	}
+}
+
 static void test_fanotify(unsigned int n)
 {
 	int ret, dirfd;
@@ -262,8 +271,7 @@ static void test_fanotify(unsigned int n)
 	 * generate FAN_CLOSE_NOWRITE event on a child, subdir or "."
 	 */
 	dirfd = SAFE_OPEN(tc->close_nowrite, O_RDONLY);
-	if (dirfd >= 0)
-		SAFE_CLOSE(dirfd);
+	SAFE_CLOSE(dirfd);
 
 	/*
 	 * First verify the first group got the file MODIFY event and got just
@@ -278,15 +286,17 @@ static void test_fanotify(unsigned int n)
 				"reading fanotify events failed");
 		}
 	}
+	event = (struct fanotify_event_metadata *)event_buf;
 	if (ret < tc->nevents * (int)FAN_EVENT_METADATA_LEN) {
-		tst_brk(TBROK,
+		tst_res(TFAIL,
 			"short read when reading fanotify events (%d < %d)",
 			ret, tc->nevents * (int)FAN_EVENT_METADATA_LEN);
 	}
-	event = (struct fanotify_event_metadata *)event_buf;
-	verify_event(0, event, FAN_MODIFY, tc->report_name ? fname : "");
-	event = FAN_EVENT_NEXT(event, ret);
-	if (tc->nevents > 1) {
+	if (FAN_EVENT_OK(event, ret)) {
+		verify_event(0, event, FAN_MODIFY, tc->report_name ? fname : "");
+		event = FAN_EVENT_NEXT(event, ret);
+	}
+	if (tc->nevents > 1 && FAN_EVENT_OK(event, ret)) {
 		verify_event(0, event, FAN_CLOSE_NOWRITE,
 			     tc->report_name ? (tc->ondir ? "." : tc->close_nowrite) : "");
 		event = FAN_EVENT_NEXT(event, ret);
@@ -296,11 +306,7 @@ static void test_fanotify(unsigned int n)
 			"first group got more than %d events (%d bytes)",
 			tc->nevents, ret);
 	}
-	/* Close all file descriptors of read events */
-	for (; FAN_EVENT_OK(event, ret); FAN_EVENT_NEXT(event, ret)) {
-		if (event->fd != FAN_NOFD)
-			SAFE_CLOSE(event->fd);
-	}
+	close_event_fds(event, ret);
 
 	/*
 	 * Then verify the rest of the groups did not get the MODIFY event and
@@ -318,15 +324,14 @@ static void test_fanotify(unsigned int n)
 			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)
-					SAFE_CLOSE(event->fd);
-			}
+			close_event_fds(event, ret);
 			continue;
 		}
 
-		if (ret == 0)
-			tst_brk(TBROK, "zero length read from fanotify fd");
+		if (ret == 0) {
+			tst_res(TFAIL, "group %d zero length read from fanotify fd", i);
+			continue;
+		}
 
 		if (errno != EAGAIN) {
 			tst_brk(TBROK | TERRNO,
@@ -360,8 +365,8 @@ static void cleanup(void)
 
 	SAFE_CHDIR("../");
 
-	if (mount_created && tst_umount(MOUNT_NAME) < 0)
-		tst_brk(TBROK | TERRNO, "umount failed");
+	if (mount_created)
+		SAFE_UMOUNT(MOUNT_NAME);
 }
 
 static struct tst_test test = {
-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/4] syscalls/fanotify09: Verify if no events are expected

[Amir Goldstein]

Some test cases expect no events for non-first groups and some expect
one event on non-dir child for non-first groups, but it is not verified
that non-first groups get the desired amount of events, so add this
information to the test case definition.

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

diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
index 60ffcb81b..a8d56c10b 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify09.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
@@ -76,6 +76,7 @@ static struct tcase {
 	unsigned int report_name;
 	const char *close_nowrite;
 	int nevents;
+	unsigned int nonfirst_event;
 } tcases[] = {
 	{
 		"Events on non-dir child with both parent and mount marks",
@@ -83,7 +84,7 @@ static struct tcase {
 		0,
 		0,
 		DIR_NAME,
-		1,
+		1, 0,
 	},
 	{
 		"Events on non-dir child and subdir with both parent and mount marks",
@@ -91,7 +92,7 @@ static struct tcase {
 		FAN_ONDIR,
 		0,
 		DIR_NAME,
-		2,
+		2, 0,
 	},
 	{
 		"Events on non-dir child and parent with both parent and mount marks",
@@ -99,7 +100,7 @@ static struct tcase {
 		FAN_ONDIR,
 		0,
 		".",
-		2,
+		2, 0
 	},
 	{
 		"Events on non-dir child and subdir with both parent and subdir marks",
@@ -107,7 +108,7 @@ static struct tcase {
 		FAN_ONDIR,
 		0,
 		DIR_NAME,
-		2,
+		2, 0,
 	},
 	{
 		"Events on non-dir children with both parent and mount marks",
@@ -115,7 +116,7 @@ static struct tcase {
 		0,
 		0,
 		FILE2_NAME,
-		2,
+		2, FAN_CLOSE_NOWRITE,
 	},
 	{
 		"Events on non-dir child with both parent and mount marks and filename info",
@@ -123,7 +124,7 @@ static struct tcase {
 		0,
 		FAN_REPORT_DFID_NAME,
 		FILE2_NAME,
-		2,
+		2, FAN_CLOSE_NOWRITE,
 	},
 };
 
@@ -315,13 +316,8 @@ static void test_fanotify(unsigned int n)
 	for (i = 1; i < NUM_GROUPS; i++) {
 		ret = read(fd_notify[i], event_buf, EVENT_BUF_LEN);
 		if (ret > 0) {
-			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, "");
+			verify_event(i, event, tc->nonfirst_event, "");
 			event = FAN_EVENT_NEXT(event, ret);
 
 			close_event_fds(event, ret);
@@ -338,7 +334,10 @@ static void test_fanotify(unsigned int n)
 				"reading fanotify events failed");
 		}
 
-		tst_res(TPASS, "group %d got no event", i);
+		if (tc->nonfirst_event)
+			tst_res(TFAIL, "group %d expected and got no event", i);
+		else
+			tst_res(TPASS, "group %d got no event as expected", i);
 	}
 	cleanup_fanotify_groups();
 }
-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/4] syscalls/fanotify09: Tidy up the test to make it more readable

[Amir Goldstein]

Document and tidy up the code dealing with mask flags FAN_ONDIR
and FAN_EVENT_ONCHILD.

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

diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
index a8d56c10b..070ad9933 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify09.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
@@ -12,21 +12,21 @@
  */
 
 /*
- * This is a regression test for commit 54a307ba8d3c:
+ * This is a regression test for commit:
  *
- *      fanotify: fix logic of events on child
+ *      54a307ba8d3c fanotify: fix logic of events on child
  *
- * Test case #1 is a regression test for commit b469e7e47c8a:
+ * Test case #1 is a regression test for commit:
  *
- *      fanotify: fix handling of events on child sub-directory
+ *      b469e7e47c8a fanotify: fix handling of events on child sub-directory
  *
- * Test case #2 is a regression test for commit 55bf882c7f13:
+ * Test case #2 is a regression test for commit:
  *
- *      fanotify: fix merging marks masks with FAN_ONDIR
+ *      55bf882c7f13 fanotify: fix merging marks masks with FAN_ONDIR
  *
- * Test case #5 is a regression test for commit 7372e79c9eb9:
+ * Test case #5 is a regression test for commit:
  *
- *      fanotify: fix logic of reporting name info with watched parent
+ *      7372e79c9eb9 fanotify: fix logic of reporting name info with watched parent
  */
 
 #define _GNU_SOURCE
@@ -131,13 +131,26 @@ static struct tcase {
 static void create_fanotify_groups(struct tcase *tc)
 {
 	struct fanotify_mark_type *mark = &tc->mark;
-	unsigned int i, onchild, report_name, ondir = tc->ondir;
+	int i;
 
 	for (i = 0; i < NUM_GROUPS; i++) {
 		/*
-		 * The first group may request events with filename info.
+		 * The first group may request events with filename info and
+		 * events on subdirs and always request events on children.
 		 */
-		report_name = (i == 0) ? tc->report_name : 0;
+		unsigned int report_name = tc->report_name;
+		unsigned int mask_flags = tc->ondir | FAN_EVENT_ON_CHILD;
+		unsigned int parent_mask;
+
+		/*
+		 * The non-first groups do not request events on children and
+		 * subdirs.
+		 */
+		if (i > 0) {
+			report_name = 0;
+			mask_flags = 0;
+		}
+
 		fd_notify[i] = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | report_name |
 						  FAN_NONBLOCK, O_RDONLY);
 
@@ -145,21 +158,20 @@ static void create_fanotify_groups(struct tcase *tc)
 		 * Add subdir or mount mark for each group with CLOSE event,
 		 * but only the first group requests events on dir.
 		 */
-		onchild = (i == 0) ? FAN_EVENT_ON_CHILD | ondir : 0;
 		SAFE_FANOTIFY_MARK(fd_notify[i],
 				    FAN_MARK_ADD | mark->flag,
-				    FAN_CLOSE_NOWRITE | onchild,
+				    FAN_CLOSE_NOWRITE | mask_flags,
 				    AT_FDCWD, tc->close_nowrite);
 
 		/*
 		 * Add inode mark on parent for each group with MODIFY event,
 		 * but only the first group requests events on child.
 		 * The one mark with FAN_EVENT_ON_CHILD is needed for
-		 * setting the DCACHE_FSNOTIFY_PARENT_WATCHED dentry
-		 * flag.
+		 * setting the DCACHE_FSNOTIFY_PARENT_WATCHED dentry flag.
 		 */
+		parent_mask = FAN_MODIFY | tc->ondir | mask_flags;
 		SAFE_FANOTIFY_MARK(fd_notify[i], FAN_MARK_ADD,
-				    FAN_MODIFY | ondir | onchild,
+				    parent_mask,
 				    AT_FDCWD, ".");
 	}
 }
-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 4/4] syscalls/fanotify09: Add test cases for merge of ignore mask

[Amir Goldstein]

1. Verify that an ignore mask that does not survive modify event,
   does survive a modify event on child, if parent is not watching
   events on children.

2. Verify that an ignore mask on parent does not ignore close events
   sent to mount mark, if parent is not watching events on children.

The behavior of these corner cases of ignore mask on parent dir have
always been undefined, so do not run the test for kernel < v5.19.

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

diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
index 070ad9933..0eb83e2f8 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify09.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
@@ -27,6 +27,11 @@
  * Test case #5 is a regression test for commit:
  *
  *      7372e79c9eb9 fanotify: fix logic of reporting name info with watched parent
+ *
+ * Test cases #6-#7 are regression tests for commit:
+ * (from v5.19, unlikely to be backported thus not in .tags):
+ *
+ *      e730558adffb fanotify: consistent behavior for parent not watching children
  */
 
 #define _GNU_SOURCE
@@ -73,6 +78,7 @@ static struct tcase {
 	const char *tname;
 	struct fanotify_mark_type mark;
 	unsigned int ondir;
+	unsigned int ignore;
 	unsigned int report_name;
 	const char *close_nowrite;
 	int nevents;
@@ -83,6 +89,7 @@ static struct tcase {
 		INIT_FANOTIFY_MARK_TYPE(MOUNT),
 		0,
 		0,
+		0,
 		DIR_NAME,
 		1, 0,
 	},
@@ -91,6 +98,7 @@ static struct tcase {
 		INIT_FANOTIFY_MARK_TYPE(MOUNT),
 		FAN_ONDIR,
 		0,
+		0,
 		DIR_NAME,
 		2, 0,
 	},
@@ -99,6 +107,7 @@ static struct tcase {
 		INIT_FANOTIFY_MARK_TYPE(MOUNT),
 		FAN_ONDIR,
 		0,
+		0,
 		".",
 		2, 0
 	},
@@ -107,6 +116,7 @@ static struct tcase {
 		INIT_FANOTIFY_MARK_TYPE(INODE),
 		FAN_ONDIR,
 		0,
+		0,
 		DIR_NAME,
 		2, 0,
 	},
@@ -115,6 +125,7 @@ static struct tcase {
 		INIT_FANOTIFY_MARK_TYPE(MOUNT),
 		0,
 		0,
+		0,
 		FILE2_NAME,
 		2, FAN_CLOSE_NOWRITE,
 	},
@@ -122,10 +133,29 @@ static struct tcase {
 		"Events on non-dir child with both parent and mount marks and filename info",
 		INIT_FANOTIFY_MARK_TYPE(MOUNT),
 		0,
+		0,
 		FAN_REPORT_DFID_NAME,
 		FILE2_NAME,
 		2, FAN_CLOSE_NOWRITE,
 	},
+	{
+		"Events on non-dir child with ignore mask on parent",
+		INIT_FANOTIFY_MARK_TYPE(MOUNT),
+		0,
+		FAN_MARK_IGNORED_MASK,
+		0,
+		DIR_NAME,
+		1, 0,
+	},
+	{
+		"Events on non-dir children with surviving ignore mask on parent",
+		INIT_FANOTIFY_MARK_TYPE(MOUNT),
+		0,
+		FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY,
+		0,
+		FILE2_NAME,
+		2, FAN_CLOSE_NOWRITE,
+	},
 };
 
 static void create_fanotify_groups(struct tcase *tc)
@@ -140,13 +170,14 @@ static void create_fanotify_groups(struct tcase *tc)
 		 */
 		unsigned int report_name = tc->report_name;
 		unsigned int mask_flags = tc->ondir | FAN_EVENT_ON_CHILD;
-		unsigned int parent_mask;
+		unsigned int parent_mask, ignore = 0;
 
 		/*
 		 * The non-first groups do not request events on children and
-		 * subdirs.
+		 * subdirs and may set an ignore mask on parent dir.
 		 */
 		if (i > 0) {
+			ignore = tc->ignore;
 			report_name = 0;
 			mask_flags = 0;
 		}
@@ -168,10 +199,15 @@ static void create_fanotify_groups(struct tcase *tc)
 		 * but only the first group requests events on child.
 		 * The one mark with FAN_EVENT_ON_CHILD is needed for
 		 * setting the DCACHE_FSNOTIFY_PARENT_WATCHED dentry flag.
+		 *
+		 * The inode mark on non-first group is either with FAN_MODIFY
+		 * in mask or FAN_CLOSE_NOWRITE in ignore mask. In either case,
+		 * it is not expected to get the modify event on a child, nor
+		 * the close event on dir.
 		 */
 		parent_mask = FAN_MODIFY | tc->ondir | mask_flags;
-		SAFE_FANOTIFY_MARK(fd_notify[i], FAN_MARK_ADD,
-				    parent_mask,
+		SAFE_FANOTIFY_MARK(fd_notify[i], FAN_MARK_ADD | ignore,
+				    ignore ? FAN_CLOSE_NOWRITE : parent_mask,
 				    AT_FDCWD, ".");
 	}
 }
@@ -186,6 +222,21 @@ static void cleanup_fanotify_groups(void)
 	}
 }
 
+static void check_ignore_mask(int fd)
+{
+	unsigned int ignored_mask, mflags;
+	char procfdinfo[100];
+
+	sprintf(procfdinfo, "/proc/%d/fdinfo/%d", (int)getpid(), fd);
+	if (FILE_LINES_SCANF(procfdinfo, "fanotify ino:%*x sdev:%*x mflags: %x mask:0 ignored_mask:%x",
+				&mflags, &ignored_mask) || !ignored_mask) {
+		tst_res(TFAIL, "The ignore mask did not survive");
+	} else {
+		tst_res(TPASS, "Found mark with ignore mask (ignored_mask=%x, mflags=%x) in %s",
+				ignored_mask, mflags, procfdinfo);
+	}
+}
+
 static void event_res(int ttype, int group,
 		      struct fanotify_event_metadata *event,
 		      const char *filename)
@@ -274,6 +325,12 @@ static void test_fanotify(unsigned int n)
 		return;
 	}
 
+	if (tc->ignore && tst_kvercmp(5, 19, 0) < 0) {
+		tst_res(TCONF, "ignored mask on parent dir has undefined "
+				"behavior on kernel < 5.19");
+		return;
+	}
+
 	create_fanotify_groups(tc);
 
 	/*
@@ -326,6 +383,13 @@ static void test_fanotify(unsigned int n)
 	 * got the FAN_CLOSE_NOWRITE event only on a non-directory.
 	 */
 	for (i = 1; i < NUM_GROUPS; i++) {
+		/*
+		 * Verify that ignore mask survived the modify event on child,
+		 * which was not supposed to be sent to this group.
+		 */
+		if (tc->ignore)
+			check_ignore_mask(fd_notify[i]);
+
 		ret = read(fd_notify[i], event_buf, EVENT_BUF_LEN);
 		if (ret > 0) {
 			event = (struct fanotify_event_metadata *)event_buf;
-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/4] syscalls/fanotify09: Cleanup open event fds on error

[Jan Kara]

On Mon 20-06-22 16:27:34, Amir Goldstein wrote:
> Avoid breaking out of test, without closing all fds of events in buffer
> to avoid failure to unmount fs on cleanup.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks good. Feel free to add:

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

							Honza

> ---
>  .../kernel/syscalls/fanotify/fanotify09.c     | 45 ++++++++++---------
>  1 file changed, 25 insertions(+), 20 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
> index fea374689..60ffcb81b 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> @@ -238,6 +238,15 @@ static void verify_event(int group, struct fanotify_event_metadata *event,
>  		SAFE_CLOSE(event->fd);
>  }
>  
> +static void close_event_fds(struct fanotify_event_metadata *event, int buflen)
> +{
> +	/* Close all file descriptors of read events */
> +	for (; FAN_EVENT_OK(event, buflen); FAN_EVENT_NEXT(event, buflen)) {
> +		if (event->fd != FAN_NOFD)
> +			SAFE_CLOSE(event->fd);
> +	}
> +}
> +
>  static void test_fanotify(unsigned int n)
>  {
>  	int ret, dirfd;
> @@ -262,8 +271,7 @@ static void test_fanotify(unsigned int n)
>  	 * generate FAN_CLOSE_NOWRITE event on a child, subdir or "."
>  	 */
>  	dirfd = SAFE_OPEN(tc->close_nowrite, O_RDONLY);
> -	if (dirfd >= 0)
> -		SAFE_CLOSE(dirfd);
> +	SAFE_CLOSE(dirfd);
>  
>  	/*
>  	 * First verify the first group got the file MODIFY event and got just
> @@ -278,15 +286,17 @@ static void test_fanotify(unsigned int n)
>  				"reading fanotify events failed");
>  		}
>  	}
> +	event = (struct fanotify_event_metadata *)event_buf;
>  	if (ret < tc->nevents * (int)FAN_EVENT_METADATA_LEN) {
> -		tst_brk(TBROK,
> +		tst_res(TFAIL,
>  			"short read when reading fanotify events (%d < %d)",
>  			ret, tc->nevents * (int)FAN_EVENT_METADATA_LEN);
>  	}
> -	event = (struct fanotify_event_metadata *)event_buf;
> -	verify_event(0, event, FAN_MODIFY, tc->report_name ? fname : "");
> -	event = FAN_EVENT_NEXT(event, ret);
> -	if (tc->nevents > 1) {
> +	if (FAN_EVENT_OK(event, ret)) {
> +		verify_event(0, event, FAN_MODIFY, tc->report_name ? fname : "");
> +		event = FAN_EVENT_NEXT(event, ret);
> +	}
> +	if (tc->nevents > 1 && FAN_EVENT_OK(event, ret)) {
>  		verify_event(0, event, FAN_CLOSE_NOWRITE,
>  			     tc->report_name ? (tc->ondir ? "." : tc->close_nowrite) : "");
>  		event = FAN_EVENT_NEXT(event, ret);
> @@ -296,11 +306,7 @@ static void test_fanotify(unsigned int n)
>  			"first group got more than %d events (%d bytes)",
>  			tc->nevents, ret);
>  	}
> -	/* Close all file descriptors of read events */
> -	for (; FAN_EVENT_OK(event, ret); FAN_EVENT_NEXT(event, ret)) {
> -		if (event->fd != FAN_NOFD)
> -			SAFE_CLOSE(event->fd);
> -	}
> +	close_event_fds(event, ret);
>  
>  	/*
>  	 * Then verify the rest of the groups did not get the MODIFY event and
> @@ -318,15 +324,14 @@ static void test_fanotify(unsigned int n)
>  			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)
> -					SAFE_CLOSE(event->fd);
> -			}
> +			close_event_fds(event, ret);
>  			continue;
>  		}
>  
> -		if (ret == 0)
> -			tst_brk(TBROK, "zero length read from fanotify fd");
> +		if (ret == 0) {
> +			tst_res(TFAIL, "group %d zero length read from fanotify fd", i);
> +			continue;
> +		}
>  
>  		if (errno != EAGAIN) {
>  			tst_brk(TBROK | TERRNO,
> @@ -360,8 +365,8 @@ static void cleanup(void)
>  
>  	SAFE_CHDIR("../");
>  
> -	if (mount_created && tst_umount(MOUNT_NAME) < 0)
> -		tst_brk(TBROK | TERRNO, "umount failed");
> +	if (mount_created)
> +		SAFE_UMOUNT(MOUNT_NAME);
>  }
>  
>  static struct tst_test test = {
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/4] syscalls/fanotify09: Verify if no events are expected

[Jan Kara]

On Mon 20-06-22 16:27:35, Amir Goldstein wrote:
> Some test cases expect no events for non-first groups and some expect
> one event on non-dir child for non-first groups, but it is not verified
> that non-first groups get the desired amount of events, so add this
> information to the test case definition.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks good. Feel free to add:

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

									Honza

> ---
>  .../kernel/syscalls/fanotify/fanotify09.c     | 25 +++++++++----------
>  1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
> index 60ffcb81b..a8d56c10b 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> @@ -76,6 +76,7 @@ static struct tcase {
>  	unsigned int report_name;
>  	const char *close_nowrite;
>  	int nevents;
> +	unsigned int nonfirst_event;
>  } tcases[] = {
>  	{
>  		"Events on non-dir child with both parent and mount marks",
> @@ -83,7 +84,7 @@ static struct tcase {
>  		0,
>  		0,
>  		DIR_NAME,
> -		1,
> +		1, 0,
>  	},
>  	{
>  		"Events on non-dir child and subdir with both parent and mount marks",
> @@ -91,7 +92,7 @@ static struct tcase {
>  		FAN_ONDIR,
>  		0,
>  		DIR_NAME,
> -		2,
> +		2, 0,
>  	},
>  	{
>  		"Events on non-dir child and parent with both parent and mount marks",
> @@ -99,7 +100,7 @@ static struct tcase {
>  		FAN_ONDIR,
>  		0,
>  		".",
> -		2,
> +		2, 0
>  	},
>  	{
>  		"Events on non-dir child and subdir with both parent and subdir marks",
> @@ -107,7 +108,7 @@ static struct tcase {
>  		FAN_ONDIR,
>  		0,
>  		DIR_NAME,
> -		2,
> +		2, 0,
>  	},
>  	{
>  		"Events on non-dir children with both parent and mount marks",
> @@ -115,7 +116,7 @@ static struct tcase {
>  		0,
>  		0,
>  		FILE2_NAME,
> -		2,
> +		2, FAN_CLOSE_NOWRITE,
>  	},
>  	{
>  		"Events on non-dir child with both parent and mount marks and filename info",
> @@ -123,7 +124,7 @@ static struct tcase {
>  		0,
>  		FAN_REPORT_DFID_NAME,
>  		FILE2_NAME,
> -		2,
> +		2, FAN_CLOSE_NOWRITE,
>  	},
>  };
>  
> @@ -315,13 +316,8 @@ static void test_fanotify(unsigned int n)
>  	for (i = 1; i < NUM_GROUPS; i++) {
>  		ret = read(fd_notify[i], event_buf, EVENT_BUF_LEN);
>  		if (ret > 0) {
> -			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, "");
> +			verify_event(i, event, tc->nonfirst_event, "");
>  			event = FAN_EVENT_NEXT(event, ret);
>  
>  			close_event_fds(event, ret);
> @@ -338,7 +334,10 @@ static void test_fanotify(unsigned int n)
>  				"reading fanotify events failed");
>  		}
>  
> -		tst_res(TPASS, "group %d got no event", i);
> +		if (tc->nonfirst_event)
> +			tst_res(TFAIL, "group %d expected and got no event", i);
> +		else
> +			tst_res(TPASS, "group %d got no event as expected", i);
>  	}
>  	cleanup_fanotify_groups();
>  }
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/4] syscalls/fanotify09: Tidy up the test to make it more readable

[Jan Kara]

On Mon 20-06-22 16:27:36, Amir Goldstein wrote:
> Document and tidy up the code dealing with mask flags FAN_ONDIR
> and FAN_EVENT_ONCHILD.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks good. Feel free to add:

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

								Honza

> ---
>  .../kernel/syscalls/fanotify/fanotify09.c     | 44 ++++++++++++-------
>  1 file changed, 28 insertions(+), 16 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
> index a8d56c10b..070ad9933 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> @@ -12,21 +12,21 @@
>   */
>  
>  /*
> - * This is a regression test for commit 54a307ba8d3c:
> + * This is a regression test for commit:
>   *
> - *      fanotify: fix logic of events on child
> + *      54a307ba8d3c fanotify: fix logic of events on child
>   *
> - * Test case #1 is a regression test for commit b469e7e47c8a:
> + * Test case #1 is a regression test for commit:
>   *
> - *      fanotify: fix handling of events on child sub-directory
> + *      b469e7e47c8a fanotify: fix handling of events on child sub-directory
>   *
> - * Test case #2 is a regression test for commit 55bf882c7f13:
> + * Test case #2 is a regression test for commit:
>   *
> - *      fanotify: fix merging marks masks with FAN_ONDIR
> + *      55bf882c7f13 fanotify: fix merging marks masks with FAN_ONDIR
>   *
> - * Test case #5 is a regression test for commit 7372e79c9eb9:
> + * Test case #5 is a regression test for commit:
>   *
> - *      fanotify: fix logic of reporting name info with watched parent
> + *      7372e79c9eb9 fanotify: fix logic of reporting name info with watched parent
>   */
>  
>  #define _GNU_SOURCE
> @@ -131,13 +131,26 @@ static struct tcase {
>  static void create_fanotify_groups(struct tcase *tc)
>  {
>  	struct fanotify_mark_type *mark = &tc->mark;
> -	unsigned int i, onchild, report_name, ondir = tc->ondir;
> +	int i;
>  
>  	for (i = 0; i < NUM_GROUPS; i++) {
>  		/*
> -		 * The first group may request events with filename info.
> +		 * The first group may request events with filename info and
> +		 * events on subdirs and always request events on children.
>  		 */
> -		report_name = (i == 0) ? tc->report_name : 0;
> +		unsigned int report_name = tc->report_name;
> +		unsigned int mask_flags = tc->ondir | FAN_EVENT_ON_CHILD;
> +		unsigned int parent_mask;
> +
> +		/*
> +		 * The non-first groups do not request events on children and
> +		 * subdirs.
> +		 */
> +		if (i > 0) {
> +			report_name = 0;
> +			mask_flags = 0;
> +		}
> +
>  		fd_notify[i] = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | report_name |
>  						  FAN_NONBLOCK, O_RDONLY);
>  
> @@ -145,21 +158,20 @@ static void create_fanotify_groups(struct tcase *tc)
>  		 * Add subdir or mount mark for each group with CLOSE event,
>  		 * but only the first group requests events on dir.
>  		 */
> -		onchild = (i == 0) ? FAN_EVENT_ON_CHILD | ondir : 0;
>  		SAFE_FANOTIFY_MARK(fd_notify[i],
>  				    FAN_MARK_ADD | mark->flag,
> -				    FAN_CLOSE_NOWRITE | onchild,
> +				    FAN_CLOSE_NOWRITE | mask_flags,
>  				    AT_FDCWD, tc->close_nowrite);
>  
>  		/*
>  		 * Add inode mark on parent for each group with MODIFY event,
>  		 * but only the first group requests events on child.
>  		 * The one mark with FAN_EVENT_ON_CHILD is needed for
> -		 * setting the DCACHE_FSNOTIFY_PARENT_WATCHED dentry
> -		 * flag.
> +		 * setting the DCACHE_FSNOTIFY_PARENT_WATCHED dentry flag.
>  		 */
> +		parent_mask = FAN_MODIFY | tc->ondir | mask_flags;
>  		SAFE_FANOTIFY_MARK(fd_notify[i], FAN_MARK_ADD,
> -				    FAN_MODIFY | ondir | onchild,
> +				    parent_mask,
>  				    AT_FDCWD, ".");
>  	}
>  }
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/4] syscalls/fanotify09: Add test cases for merge of ignore mask

[Jan Kara]

On Mon 20-06-22 16:27:37, Amir Goldstein wrote:
> 1. Verify that an ignore mask that does not survive modify event,
>    does survive a modify event on child, if parent is not watching
>    events on children.
> 
> 2. Verify that an ignore mask on parent does not ignore close events
>    sent to mount mark, if parent is not watching events on children.
> 
> The behavior of these corner cases of ignore mask on parent dir have
> always been undefined, so do not run the test for kernel < v5.19.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Hum, I was looking into the testcase. What does generate a modify event
there and checks that ignore mask does not survive it?

								Honza

> ---
>  .../kernel/syscalls/fanotify/fanotify09.c     | 72 +++++++++++++++++--
>  1 file changed, 68 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
> index 070ad9933..0eb83e2f8 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> @@ -27,6 +27,11 @@
>   * Test case #5 is a regression test for commit:
>   *
>   *      7372e79c9eb9 fanotify: fix logic of reporting name info with watched parent
> + *
> + * Test cases #6-#7 are regression tests for commit:
> + * (from v5.19, unlikely to be backported thus not in .tags):
> + *
> + *      e730558adffb fanotify: consistent behavior for parent not watching children
>   */
>  
>  #define _GNU_SOURCE
> @@ -73,6 +78,7 @@ static struct tcase {
>  	const char *tname;
>  	struct fanotify_mark_type mark;
>  	unsigned int ondir;
> +	unsigned int ignore;
>  	unsigned int report_name;
>  	const char *close_nowrite;
>  	int nevents;
> @@ -83,6 +89,7 @@ static struct tcase {
>  		INIT_FANOTIFY_MARK_TYPE(MOUNT),
>  		0,
>  		0,
> +		0,
>  		DIR_NAME,
>  		1, 0,
>  	},
> @@ -91,6 +98,7 @@ static struct tcase {
>  		INIT_FANOTIFY_MARK_TYPE(MOUNT),
>  		FAN_ONDIR,
>  		0,
> +		0,
>  		DIR_NAME,
>  		2, 0,
>  	},
> @@ -99,6 +107,7 @@ static struct tcase {
>  		INIT_FANOTIFY_MARK_TYPE(MOUNT),
>  		FAN_ONDIR,
>  		0,
> +		0,
>  		".",
>  		2, 0
>  	},
> @@ -107,6 +116,7 @@ static struct tcase {
>  		INIT_FANOTIFY_MARK_TYPE(INODE),
>  		FAN_ONDIR,
>  		0,
> +		0,
>  		DIR_NAME,
>  		2, 0,
>  	},
> @@ -115,6 +125,7 @@ static struct tcase {
>  		INIT_FANOTIFY_MARK_TYPE(MOUNT),
>  		0,
>  		0,
> +		0,
>  		FILE2_NAME,
>  		2, FAN_CLOSE_NOWRITE,
>  	},
> @@ -122,10 +133,29 @@ static struct tcase {
>  		"Events on non-dir child with both parent and mount marks and filename info",
>  		INIT_FANOTIFY_MARK_TYPE(MOUNT),
>  		0,
> +		0,
>  		FAN_REPORT_DFID_NAME,
>  		FILE2_NAME,
>  		2, FAN_CLOSE_NOWRITE,
>  	},
> +	{
> +		"Events on non-dir child with ignore mask on parent",
> +		INIT_FANOTIFY_MARK_TYPE(MOUNT),
> +		0,
> +		FAN_MARK_IGNORED_MASK,
> +		0,
> +		DIR_NAME,
> +		1, 0,
> +	},
> +	{
> +		"Events on non-dir children with surviving ignore mask on parent",
> +		INIT_FANOTIFY_MARK_TYPE(MOUNT),
> +		0,
> +		FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY,
> +		0,
> +		FILE2_NAME,
> +		2, FAN_CLOSE_NOWRITE,
> +	},
>  };
>  
>  static void create_fanotify_groups(struct tcase *tc)
> @@ -140,13 +170,14 @@ static void create_fanotify_groups(struct tcase *tc)
>  		 */
>  		unsigned int report_name = tc->report_name;
>  		unsigned int mask_flags = tc->ondir | FAN_EVENT_ON_CHILD;
> -		unsigned int parent_mask;
> +		unsigned int parent_mask, ignore = 0;
>  
>  		/*
>  		 * The non-first groups do not request events on children and
> -		 * subdirs.
> +		 * subdirs and may set an ignore mask on parent dir.
>  		 */
>  		if (i > 0) {
> +			ignore = tc->ignore;
>  			report_name = 0;
>  			mask_flags = 0;
>  		}
> @@ -168,10 +199,15 @@ static void create_fanotify_groups(struct tcase *tc)
>  		 * but only the first group requests events on child.
>  		 * The one mark with FAN_EVENT_ON_CHILD is needed for
>  		 * setting the DCACHE_FSNOTIFY_PARENT_WATCHED dentry flag.
> +		 *
> +		 * The inode mark on non-first group is either with FAN_MODIFY
> +		 * in mask or FAN_CLOSE_NOWRITE in ignore mask. In either case,
> +		 * it is not expected to get the modify event on a child, nor
> +		 * the close event on dir.
>  		 */
>  		parent_mask = FAN_MODIFY | tc->ondir | mask_flags;
> -		SAFE_FANOTIFY_MARK(fd_notify[i], FAN_MARK_ADD,
> -				    parent_mask,
> +		SAFE_FANOTIFY_MARK(fd_notify[i], FAN_MARK_ADD | ignore,
> +				    ignore ? FAN_CLOSE_NOWRITE : parent_mask,
>  				    AT_FDCWD, ".");
>  	}
>  }
> @@ -186,6 +222,21 @@ static void cleanup_fanotify_groups(void)
>  	}
>  }
>  
> +static void check_ignore_mask(int fd)
> +{
> +	unsigned int ignored_mask, mflags;
> +	char procfdinfo[100];
> +
> +	sprintf(procfdinfo, "/proc/%d/fdinfo/%d", (int)getpid(), fd);
> +	if (FILE_LINES_SCANF(procfdinfo, "fanotify ino:%*x sdev:%*x mflags: %x mask:0 ignored_mask:%x",
> +				&mflags, &ignored_mask) || !ignored_mask) {
> +		tst_res(TFAIL, "The ignore mask did not survive");
> +	} else {
> +		tst_res(TPASS, "Found mark with ignore mask (ignored_mask=%x, mflags=%x) in %s",
> +				ignored_mask, mflags, procfdinfo);
> +	}
> +}
> +
>  static void event_res(int ttype, int group,
>  		      struct fanotify_event_metadata *event,
>  		      const char *filename)
> @@ -274,6 +325,12 @@ static void test_fanotify(unsigned int n)
>  		return;
>  	}
>  
> +	if (tc->ignore && tst_kvercmp(5, 19, 0) < 0) {
> +		tst_res(TCONF, "ignored mask on parent dir has undefined "
> +				"behavior on kernel < 5.19");
> +		return;
> +	}
> +
>  	create_fanotify_groups(tc);
>  
>  	/*
> @@ -326,6 +383,13 @@ static void test_fanotify(unsigned int n)
>  	 * got the FAN_CLOSE_NOWRITE event only on a non-directory.
>  	 */
>  	for (i = 1; i < NUM_GROUPS; i++) {
> +		/*
> +		 * Verify that ignore mask survived the modify event on child,
> +		 * which was not supposed to be sent to this group.
> +		 */
> +		if (tc->ignore)
> +			check_ignore_mask(fd_notify[i]);
> +
>  		ret = read(fd_notify[i], event_buf, EVENT_BUF_LEN);
>  		if (ret > 0) {
>  			event = (struct fanotify_event_metadata *)event_buf;
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/4] syscalls/fanotify09: Add test cases for merge of ignore mask

[Amir Goldstein]

On Mon, Jun 20, 2022 at 6:20 PM Jan Kara <jack@suse.cz> wrote:
>
> On Mon 20-06-22 16:27:37, Amir Goldstein wrote:
> > 1. Verify that an ignore mask that does not survive modify event,
> >    does survive a modify event on child, if parent is not watching
> >    events on children.
> >
> > 2. Verify that an ignore mask on parent does not ignore close events
> >    sent to mount mark, if parent is not watching events on children.
> >
> > The behavior of these corner cases of ignore mask on parent dir have
> > always been undefined, so do not run the test for kernel < v5.19.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> Hum, I was looking into the testcase. What does generate a modify event
> there and checks that ignore mask does not survive it?

I think this does:

        /*
         * generate MODIFY event and no FAN_CLOSE_NOWRITE event.
         */
        SAFE_FILE_PRINTF(fname, "1");

Thanks,
Amir.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/4] syscalls/fanotify09: Add test cases for merge of ignore mask

[Jan Kara]

On Mon 20-06-22 19:59:48, Amir Goldstein wrote:
> On Mon, Jun 20, 2022 at 6:20 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Mon 20-06-22 16:27:37, Amir Goldstein wrote:
> > > 1. Verify that an ignore mask that does not survive modify event,
> > >    does survive a modify event on child, if parent is not watching
> > >    events on children.
> > >
> > > 2. Verify that an ignore mask on parent does not ignore close events
> > >    sent to mount mark, if parent is not watching events on children.
> > >
> > > The behavior of these corner cases of ignore mask on parent dir have
> > > always been undefined, so do not run the test for kernel < v5.19.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >
> > Hum, I was looking into the testcase. What does generate a modify event
> > there and checks that ignore mask does not survive it?
> 
> I think this does:
> 
>         /*
>          * generate MODIFY event and no FAN_CLOSE_NOWRITE event.
>          */
>         SAFE_FILE_PRINTF(fname, "1");

Yeah, I was looking at that but then I'm somewhat confused because this
gets called in .setup() hook while the notification groups get created only
in .test() hook that gets called later. What am I missing?

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/4] syscalls/fanotify09: Add test cases for merge of ignore mask

[Amir Goldstein]

On Mon, Jun 20, 2022 at 11:35 PM Jan Kara <jack@suse.cz> wrote:
>
> On Mon 20-06-22 19:59:48, Amir Goldstein wrote:
> > On Mon, Jun 20, 2022 at 6:20 PM Jan Kara <jack@suse.cz> wrote:
> > >
> > > On Mon 20-06-22 16:27:37, Amir Goldstein wrote:
> > > > 1. Verify that an ignore mask that does not survive modify event,
> > > >    does survive a modify event on child, if parent is not watching
> > > >    events on children.
> > > >
> > > > 2. Verify that an ignore mask on parent does not ignore close events
> > > >    sent to mount mark, if parent is not watching events on children.
> > > >
> > > > The behavior of these corner cases of ignore mask on parent dir have
> > > > always been undefined, so do not run the test for kernel < v5.19.
> > > >
> > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > >
> > > Hum, I was looking into the testcase. What does generate a modify event
> > > there and checks that ignore mask does not survive it?
> >
> > I think this does:
> >
> >         /*
> >          * generate MODIFY event and no FAN_CLOSE_NOWRITE event.
> >          */
> >         SAFE_FILE_PRINTF(fname, "1");
>
> Yeah, I was looking at that but then I'm somewhat confused because this
> gets called in .setup() hook while the notification groups get created only
> in .test() hook that gets called later. What am I missing?

There is one in setup().
The one with the comment above is from test_fanotify().

Thanks,
Amir.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/4] syscalls/fanotify09: Add test cases for merge of ignore mask

[Jan Kara]

On Tue 21-06-22 06:02:24, Amir Goldstein wrote:
> On Mon, Jun 20, 2022 at 11:35 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Mon 20-06-22 19:59:48, Amir Goldstein wrote:
> > > On Mon, Jun 20, 2022 at 6:20 PM Jan Kara <jack@suse.cz> wrote:
> > > >
> > > > On Mon 20-06-22 16:27:37, Amir Goldstein wrote:
> > > > > 1. Verify that an ignore mask that does not survive modify event,
> > > > >    does survive a modify event on child, if parent is not watching
> > > > >    events on children.
> > > > >
> > > > > 2. Verify that an ignore mask on parent does not ignore close events
> > > > >    sent to mount mark, if parent is not watching events on children.
> > > > >
> > > > > The behavior of these corner cases of ignore mask on parent dir have
> > > > > always been undefined, so do not run the test for kernel < v5.19.
> > > > >
> > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > >
> > > > Hum, I was looking into the testcase. What does generate a modify event
> > > > there and checks that ignore mask does not survive it?
> > >
> > > I think this does:
> > >
> > >         /*
> > >          * generate MODIFY event and no FAN_CLOSE_NOWRITE event.
> > >          */
> > >         SAFE_FILE_PRINTF(fname, "1");
> >
> > Yeah, I was looking at that but then I'm somewhat confused because this
> > gets called in .setup() hook while the notification groups get created only
> > in .test() hook that gets called later. What am I missing?
> 
> There is one in setup().
> The one with the comment above is from test_fanotify().

Bah, I must have been blind... Thanks for directing me.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/4] syscalls/fanotify09: Add test cases for merge of ignore mask

[Jan Kara]

On Mon 20-06-22 16:27:37, Amir Goldstein wrote:
> 1. Verify that an ignore mask that does not survive modify event,
>    does survive a modify event on child, if parent is not watching
>    events on children.
> 
> 2. Verify that an ignore mask on parent does not ignore close events
>    sent to mount mark, if parent is not watching events on children.
> 
> The behavior of these corner cases of ignore mask on parent dir have
> always been undefined, so do not run the test for kernel < v5.19.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

OK, after Amir cleared my confusing things looks good to me. Feel free to
add:

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

								Honza

> ---
>  .../kernel/syscalls/fanotify/fanotify09.c     | 72 +++++++++++++++++--
>  1 file changed, 68 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
> index 070ad9933..0eb83e2f8 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> @@ -27,6 +27,11 @@
>   * Test case #5 is a regression test for commit:
>   *
>   *      7372e79c9eb9 fanotify: fix logic of reporting name info with watched parent
> + *
> + * Test cases #6-#7 are regression tests for commit:
> + * (from v5.19, unlikely to be backported thus not in .tags):
> + *
> + *      e730558adffb fanotify: consistent behavior for parent not watching children
>   */
>  
>  #define _GNU_SOURCE
> @@ -73,6 +78,7 @@ static struct tcase {
>  	const char *tname;
>  	struct fanotify_mark_type mark;
>  	unsigned int ondir;
> +	unsigned int ignore;
>  	unsigned int report_name;
>  	const char *close_nowrite;
>  	int nevents;
> @@ -83,6 +89,7 @@ static struct tcase {
>  		INIT_FANOTIFY_MARK_TYPE(MOUNT),
>  		0,
>  		0,
> +		0,
>  		DIR_NAME,
>  		1, 0,
>  	},
> @@ -91,6 +98,7 @@ static struct tcase {
>  		INIT_FANOTIFY_MARK_TYPE(MOUNT),
>  		FAN_ONDIR,
>  		0,
> +		0,
>  		DIR_NAME,
>  		2, 0,
>  	},
> @@ -99,6 +107,7 @@ static struct tcase {
>  		INIT_FANOTIFY_MARK_TYPE(MOUNT),
>  		FAN_ONDIR,
>  		0,
> +		0,
>  		".",
>  		2, 0
>  	},
> @@ -107,6 +116,7 @@ static struct tcase {
>  		INIT_FANOTIFY_MARK_TYPE(INODE),
>  		FAN_ONDIR,
>  		0,
> +		0,
>  		DIR_NAME,
>  		2, 0,
>  	},
> @@ -115,6 +125,7 @@ static struct tcase {
>  		INIT_FANOTIFY_MARK_TYPE(MOUNT),
>  		0,
>  		0,
> +		0,
>  		FILE2_NAME,
>  		2, FAN_CLOSE_NOWRITE,
>  	},
> @@ -122,10 +133,29 @@ static struct tcase {
>  		"Events on non-dir child with both parent and mount marks and filename info",
>  		INIT_FANOTIFY_MARK_TYPE(MOUNT),
>  		0,
> +		0,
>  		FAN_REPORT_DFID_NAME,
>  		FILE2_NAME,
>  		2, FAN_CLOSE_NOWRITE,
>  	},
> +	{
> +		"Events on non-dir child with ignore mask on parent",
> +		INIT_FANOTIFY_MARK_TYPE(MOUNT),
> +		0,
> +		FAN_MARK_IGNORED_MASK,
> +		0,
> +		DIR_NAME,
> +		1, 0,
> +	},
> +	{
> +		"Events on non-dir children with surviving ignore mask on parent",
> +		INIT_FANOTIFY_MARK_TYPE(MOUNT),
> +		0,
> +		FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY,
> +		0,
> +		FILE2_NAME,
> +		2, FAN_CLOSE_NOWRITE,
> +	},
>  };
>  
>  static void create_fanotify_groups(struct tcase *tc)
> @@ -140,13 +170,14 @@ static void create_fanotify_groups(struct tcase *tc)
>  		 */
>  		unsigned int report_name = tc->report_name;
>  		unsigned int mask_flags = tc->ondir | FAN_EVENT_ON_CHILD;
> -		unsigned int parent_mask;
> +		unsigned int parent_mask, ignore = 0;
>  
>  		/*
>  		 * The non-first groups do not request events on children and
> -		 * subdirs.
> +		 * subdirs and may set an ignore mask on parent dir.
>  		 */
>  		if (i > 0) {
> +			ignore = tc->ignore;
>  			report_name = 0;
>  			mask_flags = 0;
>  		}
> @@ -168,10 +199,15 @@ static void create_fanotify_groups(struct tcase *tc)
>  		 * but only the first group requests events on child.
>  		 * The one mark with FAN_EVENT_ON_CHILD is needed for
>  		 * setting the DCACHE_FSNOTIFY_PARENT_WATCHED dentry flag.
> +		 *
> +		 * The inode mark on non-first group is either with FAN_MODIFY
> +		 * in mask or FAN_CLOSE_NOWRITE in ignore mask. In either case,
> +		 * it is not expected to get the modify event on a child, nor
> +		 * the close event on dir.
>  		 */
>  		parent_mask = FAN_MODIFY | tc->ondir | mask_flags;
> -		SAFE_FANOTIFY_MARK(fd_notify[i], FAN_MARK_ADD,
> -				    parent_mask,
> +		SAFE_FANOTIFY_MARK(fd_notify[i], FAN_MARK_ADD | ignore,
> +				    ignore ? FAN_CLOSE_NOWRITE : parent_mask,
>  				    AT_FDCWD, ".");
>  	}
>  }
> @@ -186,6 +222,21 @@ static void cleanup_fanotify_groups(void)
>  	}
>  }
>  
> +static void check_ignore_mask(int fd)
> +{
> +	unsigned int ignored_mask, mflags;
> +	char procfdinfo[100];
> +
> +	sprintf(procfdinfo, "/proc/%d/fdinfo/%d", (int)getpid(), fd);
> +	if (FILE_LINES_SCANF(procfdinfo, "fanotify ino:%*x sdev:%*x mflags: %x mask:0 ignored_mask:%x",
> +				&mflags, &ignored_mask) || !ignored_mask) {
> +		tst_res(TFAIL, "The ignore mask did not survive");
> +	} else {
> +		tst_res(TPASS, "Found mark with ignore mask (ignored_mask=%x, mflags=%x) in %s",
> +				ignored_mask, mflags, procfdinfo);
> +	}
> +}
> +
>  static void event_res(int ttype, int group,
>  		      struct fanotify_event_metadata *event,
>  		      const char *filename)
> @@ -274,6 +325,12 @@ static void test_fanotify(unsigned int n)
>  		return;
>  	}
>  
> +	if (tc->ignore && tst_kvercmp(5, 19, 0) < 0) {
> +		tst_res(TCONF, "ignored mask on parent dir has undefined "
> +				"behavior on kernel < 5.19");
> +		return;
> +	}
> +
>  	create_fanotify_groups(tc);
>  
>  	/*
> @@ -326,6 +383,13 @@ static void test_fanotify(unsigned int n)
>  	 * got the FAN_CLOSE_NOWRITE event only on a non-directory.
>  	 */
>  	for (i = 1; i < NUM_GROUPS; i++) {
> +		/*
> +		 * Verify that ignore mask survived the modify event on child,
> +		 * which was not supposed to be sent to this group.
> +		 */
> +		if (tc->ignore)
> +			check_ignore_mask(fd_notify[i]);
> +
>  		ret = read(fd_notify[i], event_buf, EVENT_BUF_LEN);
>  		if (ret > 0) {
>  			event = (struct fanotify_event_metadata *)event_buf;
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/4] syscalls/fanotify09: Add test cases for merge of ignore mask

[Petr Vorel]

Hi Amir, Jan,

> OK, after Amir cleared my confusing things looks good to me. Feel free to
> add:

Thanks for your work on fanotify tests.
Patches are good as always, merged.

Kind regards,
Petr

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

> 								Honza

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp