fanotify filesystem mark tests

PATCH New Amir Goldstein 2 participants 17 messages Nov 16, 2018 – Nov 22, 2018

lore

Share & tools

lore

Amir Goldstein

author

2018-11-16 06:51 UTC #0 lore raw

cover fanotify filesystem mark tests

Hi Cyril,

This series adds some new tests to fanotify and is also available on my github tree [1].

Patches 1-2 re-structure tests fanotify{01,03} to iterate over the fanotify mark types and add some missing test coverage for FAN_MARK_MOUNT.

Patches 3-5 add test coverage for the new FAN_MARK_FILESYSTEM API, that was merged for kernel v4.20-rc1, to tests fanotify{01,03,10}. Older kernels will report TCONF on the new test cases.

Previous patches from me pending your review:
- Add test case to fanotify09 to track a bug fix in linux-next [2]
- Enhance readahead02 to track an overlayfs regression fix in v4.19 [3]

Thanks,
Amir.

[1] https://github.com/amir73il/ltp/commits/fanotify_sb
[2] https://github.com/amir73il/ltp/commits/fanotify_ignore
[3] https://github.com/amir73il/ltp/commits/readahead_fadvise

Amir Goldstein (5):
syscalls/fanotify01: check events also on mountpoint mark syscalls/fanotify03: check permission events also on mountpoint mark syscalls/fanotify01: add test for FAN_MARK_FILESYSTEM events syscalls/fanotify03: add test for FAN_MARK_FILESYSTEM permission
events
syscalls/fanotify10: add tests for FAN_MARK_FILESYSTEM and ignore mask

 testcases/kernel/syscalls/fanotify/fanotify.h |  3 +
 .../kernel/syscalls/fanotify/fanotify01.c     | 92 ++++++++++++-------
 .../kernel/syscalls/fanotify/fanotify03.c     | 55 +++++++++--
 .../kernel/syscalls/fanotify/fanotify10.c     | 40 +++++++-
 4 files changed, 148 insertions(+), 42 deletions(-)

Signature · 1 line

2.17.1

Amir Goldstein

author

2018-11-16 06:51 UTC #1 lore raw

PATCH 1/5 syscalls/fanotify01: check events also on mountpoint mark

Add index to test and repeat it for mark types inode and mountpoint. Move fanotify_init() into the test to simplify setup/cleanup.

Signed-off-by Amir Goldstein <[email protected]>

---
 .../kernel/syscalls/fanotify/fanotify01.c     | 83 ++++++++++++-------
 1 file changed, 51 insertions(+), 32 deletions(-)

testcases/kernel/syscalls/fanotify/fanotify01.c +51 ~~-32~~

index e1210134b..5dfb67b61 100644
@@ -32,26 +32,46 @@
 #define BUF_SIZE 256
 #define TST_TOTAL 12
 
+static struct tcase {
+	const char *tname;
+	struct fanotify_mark_type mark;
+} tcases[] = {
+	{
+		"inode mark events",
+		INIT_FANOTIFY_MARK_TYPE(INODE),
+	},
+	{
+		"mount mark events",
+		INIT_FANOTIFY_MARK_TYPE(MOUNT),
+	},
+};
+
 static char fname[BUF_SIZE];
 static char buf[BUF_SIZE];
-static int fd, fd_notify;
+static int fd_notify;
 
 static unsigned long long event_set[EVENT_MAX];
 
 static char event_buf[EVENT_BUF_LEN];
 
-void test01(void)
+static void test_fanotify(unsigned int n)
 {
-	int ret, len, i = 0, test_num = 0;
-
+	struct tcase *tc = &tcases[n];
+	struct fanotify_mark_type *mark = &tc->mark;
+	int fd, ret, len, i = 0, test_num = 0;
 	int tst_count = 0;
 
-	if (fanotify_mark(fd_notify, FAN_MARK_ADD, FAN_ACCESS | FAN_MODIFY |
-			    FAN_CLOSE | FAN_OPEN, AT_FDCWD, fname) < 0) {
+	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
+
+	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
+
+	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
+			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
+			  AT_FDCWD, fname) < 0) {
 		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS | "
+			"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS | %s | "
 			"FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, %s) "
-			"failed", fd_notify, fname);
+			"failed", fd_notify, mark->name, fname);
 	}
 
 	/*

@@ -101,12 +121,12 @@ void test01(void)
 
 	/* Ignore access events */
 	if (fanotify_mark(fd_notify,
-			  FAN_MARK_ADD | FAN_MARK_IGNORED_MASK,
+			  FAN_MARK_ADD | mark->flag | FAN_MARK_IGNORED_MASK,
 			  FAN_ACCESS, AT_FDCWD, fname) < 0) {
 		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD | "
-			"FAN_MARK_IGNORED_MASK, FAN_ACCESS, "
-			"AT_FDCWD, %s) failed", fd_notify, fname);
+			"fanotify_mark (%d, FAN_MARK_ADD | %s | "
+			"FAN_MARK_IGNORED_MASK, FAN_ACCESS, AT_FDCWD, %s) "
+			"failed", fd_notify, mark->name, fname);
 	}
 
 	fd = SAFE_OPEN(fname, O_RDWR);

@@ -150,15 +170,14 @@ void test01(void)
 	 * Now ignore open & close events regardless of file
 	 * modifications
 	 */
-	if (fanotify_mark(fd_notify,
-			    FAN_MARK_ADD | FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY,
-			    FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname) < 0) {
+	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag |
+			  FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY,
+			  FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname) < 0) {
 		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_ADD | "
-			"FAN_MARK_IGNORED_MASK | "
-			"FAN_MARK_IGNORED_SURV_MODIFY, FAN_OPEN | "
-			"FAN_CLOSE, AT_FDCWD, %s) failed", fd_notify,
-			fname);
+			"fanotify_mark (%d, FAN_MARK_ADD | %s | "
+			"FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY, "
+			"FAN_OPEN | FAN_CLOSE, AT_FDCWD, %s) failed",
+			fd_notify, mark->name, fname);
 	}
 
 	/* This event should be ignored */

@@ -181,13 +200,13 @@ void test01(void)
 
 	/* Now remove open and close from ignored mask */
 	if (fanotify_mark(fd_notify,
-			  FAN_MARK_REMOVE | FAN_MARK_IGNORED_MASK,
+			  FAN_MARK_REMOVE | mark->flag | FAN_MARK_IGNORED_MASK,
 			  FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname) < 0) {
 		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_REMOVE | "
-			"FAN_MARK_IGNORED_MASK, FAN_OPEN | "
-			"FAN_CLOSE, AT_FDCWD, %s) failed", fd_notify,
-			fname);
+			"fanotify_mark (%d, FAN_MARK_REMOVE | %s | "
+			"FAN_MARK_IGNORED_MASK, FAN_OPEN | FAN_CLOSE, "
+			"AT_FDCWD, %s) failed", fd_notify,
+			mark->name, fname);
 	}
 
 	SAFE_CLOSE(fd);

@@ -283,12 +302,13 @@ pass:
 
 	}
 	/* Remove mark to clear FAN_MARK_IGNORED_SURV_MODIFY */
-	if (fanotify_mark(fd_notify, FAN_MARK_REMOVE, FAN_ACCESS | FAN_MODIFY |
-			    FAN_CLOSE | FAN_OPEN, AT_FDCWD, fname) < 0) {
+	if (fanotify_mark(fd_notify, FAN_MARK_REMOVE | mark->flag,
+			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
+			  AT_FDCWD, fname) < 0) {
 		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_REMOVE, FAN_ACCESS | "
+			"fanotify_mark (%d, FAN_MARK_REMOVE | %s, FAN_ACCESS | "
 			"FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, %s) "
-			"failed", fd_notify, fname);
+			"failed", fd_notify, mark->name, fname);
 	}
 }

@@ -296,8 +316,6 @@ static void setup(void)
 {
 	sprintf(fname, "tfile_%d", getpid());
 	SAFE_FILE_PRINTF(fname, "1");
-
-	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
 }
 
 static void cleanup(void)

@@ -307,7 +325,8 @@ static void cleanup(void)
 }
 
 static struct tst_test test = {
-	.test_all = test01,
+	.test = test_fanotify,
+	.tcnt = ARRAY_SIZE(tcases),
 	.setup = setup,
 	.cleanup = cleanup,
 	.needs_tmpdir = 1,

Signature · 1 line

2.17.1

Cyril Hrubis

2018-11-21 12:01 UTC #2 lore raw

PATCH 1/5 syscalls/fanotify01: check events also on mountpoint mark

Hi!

Quoted 39 lines

testcases/kernel/syscalls/fanotify/fanotify01.c +10 -9

 	/* This event should be ignored */

@@ -181,13 +200,13 @@ void test01(void)
 
 	/* Now remove open and close from ignored mask */
 	if (fanotify_mark(fd_notify,
-			  FAN_MARK_REMOVE | FAN_MARK_IGNORED_MASK,
+			  FAN_MARK_REMOVE | mark->flag | FAN_MARK_IGNORED_MASK,
 			  FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname) < 0) {
 		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_REMOVE | "
-			"FAN_MARK_IGNORED_MASK, FAN_OPEN | "
-			"FAN_CLOSE, AT_FDCWD, %s) failed", fd_notify,
-			fname);
+			"fanotify_mark (%d, FAN_MARK_REMOVE | %s | "
+			"FAN_MARK_IGNORED_MASK, FAN_OPEN | FAN_CLOSE, "
+			"AT_FDCWD, %s) failed", fd_notify,
+			mark->name, fname);
 	}
 
 	SAFE_CLOSE(fd);

@@ -283,12 +302,13 @@ pass:
 
 	}
 	/* Remove mark to clear FAN_MARK_IGNORED_SURV_MODIFY */
-	if (fanotify_mark(fd_notify, FAN_MARK_REMOVE, FAN_ACCESS | FAN_MODIFY |
-			    FAN_CLOSE | FAN_OPEN, AT_FDCWD, fname) < 0) {
+	if (fanotify_mark(fd_notify, FAN_MARK_REMOVE | mark->flag,
+			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
+			  AT_FDCWD, fname) < 0) {
 		tst_brk(TBROK | TERRNO,
-			"fanotify_mark (%d, FAN_MARK_REMOVE, FAN_ACCESS | "
+			"fanotify_mark (%d, FAN_MARK_REMOVE | %s, FAN_ACCESS | "
 			"FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, %s) "
-			"failed", fd_notify, fname);
+			"failed", fd_notify, mark->name, fname);
 	}
 }

testcases/kernel/syscalls/fanotify/fanotify01.c static void setup(void) · line 300

Quoted 5 lines

testcases/kernel/syscalls/fanotify/fanotify01.c +0 -2

@@ -296,8 +316,6 @@ static void setup(void)
 {
 	sprintf(fname, "tfile_%d", getpid());
 	SAFE_FILE_PRINTF(fname, "1");
-
-	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);

I've added SAFE_CLOSE(fd_notify) here so that we do not use all available file descriptors with large enough -i parameter and pushed.

I do wonder what is the reason to move the fanotify_init() from setup the the test_fanotify(), is there a problem with recycling the same fd for different tests?

Signature · 2 lines

Cyril Hrubis
[email protected]

Amir Goldstein

author

2018-11-21 16:18 UTC #3 lore raw

PATCH 1/5 syscalls/fanotify01: check events also on mountpoint mark

Quoted 50 lines · Cyril Hrubis

Hi!

Quoted 39 lines

      /* This event should be ignored */

testcases/kernel/syscalls/fanotify/fanotify01.c +10 -9

@@ -181,13 +200,13 @@ void test01(void)
 
      /* Now remove open and close from ignored mask */
      if (fanotify_mark(fd_notify,
-                       FAN_MARK_REMOVE | FAN_MARK_IGNORED_MASK,
+                       FAN_MARK_REMOVE | mark->flag | FAN_MARK_IGNORED_MASK,
                        FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname) < 0) {
              tst_brk(TBROK | TERRNO,
-                     "fanotify_mark (%d, FAN_MARK_REMOVE | "
-                     "FAN_MARK_IGNORED_MASK, FAN_OPEN | "
-                     "FAN_CLOSE, AT_FDCWD, %s) failed", fd_notify,
-                     fname);
+                     "fanotify_mark (%d, FAN_MARK_REMOVE | %s | "
+                     "FAN_MARK_IGNORED_MASK, FAN_OPEN | FAN_CLOSE, "
+                     "AT_FDCWD, %s) failed", fd_notify,
+                     mark->name, fname);
      }
 
      SAFE_CLOSE(fd);

@@ -283,12 +302,13 @@ pass:
 
      }
      /* Remove mark to clear FAN_MARK_IGNORED_SURV_MODIFY */
-     if (fanotify_mark(fd_notify, FAN_MARK_REMOVE, FAN_ACCESS | FAN_MODIFY |
-                         FAN_CLOSE | FAN_OPEN, AT_FDCWD, fname) < 0) {
+     if (fanotify_mark(fd_notify, FAN_MARK_REMOVE | mark->flag,
+                       FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
+                       AT_FDCWD, fname) < 0) {
              tst_brk(TBROK | TERRNO,
-                     "fanotify_mark (%d, FAN_MARK_REMOVE, FAN_ACCESS | "
+                     "fanotify_mark (%d, FAN_MARK_REMOVE | %s, FAN_ACCESS | "
                      "FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, %s) "
-                     "failed", fd_notify, fname);
+                     "failed", fd_notify, mark->name, fname);
      }
 }

testcases/kernel/syscalls/fanotify/fanotify01.c static void setup(void) · line 300

Quoted 5 lines

testcases/kernel/syscalls/fanotify/fanotify01.c +0 -2

@@ -296,8 +316,6 @@ static void setup(void)
 {
      sprintf(fname, "tfile_%d", getpid());
      SAFE_FILE_PRINTF(fname, "1");
-
-     fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);

I've added SAFE_CLOSE(fd_notify) here so that we do not use all available file descriptors with large enough -i parameter and pushed.

Quoted 4 lines

I do wonder what is the reason to move the fanotify_init() from setup the the test_fanotify(), is there a problem with recycling the same fd for different tests?

Closing fd_notify is an easier and better cleanup between test cases. Otherwise we would need to FAN_MARK_FLUSH all the individual marks that we added.

Thanks,
Amir.

Cyril Hrubis

2018-11-22 16:06 UTC #4 lore raw

PATCH 1/5 syscalls/fanotify01: check events also on mountpoint mark

Hi!

Quoted 11 lines

Quoted 7 lines
I've added SAFE_CLOSE(fd_notify) here so that we do not use all available file descriptors with large enough -i parameter and pushed.
I do wonder what is the reason to move the fanotify_init() from setup the the test_fanotify(), is there a problem with recycling the same fd for different tests?
Closing fd_notify is an easier and better cleanup between test cases. Otherwise we would need to FAN_MARK_FLUSH all the individual marks that we added.

Wouldn't that cover more of kernel codepaths? If we reused the fd and cleared all the marks we would test that the API for clearing the marks works as expected. Or should we write a separate test for that?

Signature · 2 lines

Cyril Hrubis
[email protected]

Amir Goldstein

author

2018-11-22 16:57 UTC #5 lore raw

PATCH 1/5 syscalls/fanotify01: check events also on mountpoint mark

Quoted 10 lines · Cyril Hrubis

Hi!
Quoted 8 lines
Quoted 7 lines
I've added SAFE_CLOSE(fd_notify) here so that we do not use all available file descriptors with large enough -i parameter and pushed.
I do wonder what is the reason to move the fanotify_init() from setup the the test_fanotify(), is there a problem with recycling the same fd for different tests?

Quoted 7 lines

Quoted 3 lines
Closing fd_notify is an easier and better cleanup between test cases. Otherwise we would need to FAN_MARK_FLUSH all the individual marks that we added.
Wouldn't that cover more of kernel codepaths? If we reused the fd and cleared all the marks we would test that the API for clearing the marks works as expected. Or should we write a separate test for that?

I see that fanotify04 exercises FAN_MARK_FLUSH. Besides it is probably the less common case (??).
Most of the time I imagine that a monitoring program sets a mark and watches events until it closes the fd.

Thanks,
Amir.

Amir Goldstein

author

2018-11-16 06:51 UTC #6 lore raw

PATCH 2/5 syscalls/fanotify03: check permission events also on mountpoint mark

In reply to Amir Goldstein

Add index to test and repeat it for mark types inode and mountpoint.

Signed-off-by Amir Goldstein <[email protected]>

---
 .../kernel/syscalls/fanotify/fanotify03.c     | 34 +++++++++++++++----
 1 file changed, 27 insertions(+), 7 deletions(-)

testcases/kernel/syscalls/fanotify/fanotify03.c +27 -7

index 42aac813e..19daf57ef 100644
@@ -47,6 +47,20 @@ static unsigned int event_resp[EVENT_MAX];
 
 static char event_buf[EVENT_BUF_LEN];
 
+static struct tcase {
+	const char *tname;
+	struct fanotify_mark_type mark;
+} tcases[] = {
+	{
+		"inode mark permission events",
+		INIT_FANOTIFY_MARK_TYPE(INODE),
+	},
+	{
+		"mount mark permission events",
+		INIT_FANOTIFY_MARK_TYPE(MOUNT),
+	},
+};
+
 static void generate_events(void)
 {
 	int fd;

@@ -120,11 +134,14 @@ static void check_child(void)
 		tst_res(TFAIL, "child %s", tst_strstatus(child_ret));
 }
 
-static void setup_mark(void)
+static void setup_mark(unsigned int n)
 {
+	struct tcase *tc = &tcases[n];
+	struct fanotify_mark_type *mark = &tc->mark;
+
 	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
 
-	if (fanotify_mark(fd_notify, FAN_MARK_ADD,
+	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
 			  FAN_ACCESS_PERM | FAN_OPEN_PERM,
 			  AT_FDCWD, fname) < 0) {
 		if (errno == EINVAL) {

@@ -133,20 +150,22 @@ static void setup_mark(void)
 				"configured in kernel?");
 		} else {
 			tst_brk(TBROK | TERRNO,
-				"fanotify_mark (%d, FAN_MARK_ADD, "
+				"fanotify_mark (%d, FAN_MARK_ADD | %s, "
 				"FAN_ACCESS_PERM | FAN_OPEN_PERM, "
 				"AT_FDCWD, %s) failed.",
-				fd_notify, fname);
+				fd_notify, mark->name, fname);
 		}
 	}
+
+	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
 }
 
-void test01(void)
+static void test_fanotify(unsigned int n)
 {
 	int tst_count;
 	int ret, len = 0, i = 0, test_num = 0;
 
-	setup_mark();
+	setup_mark(n);
 	run_child();
 
 	tst_count = 0;

@@ -247,7 +266,8 @@ static void cleanup(void)
 }
 
 static struct tst_test test = {
-	.test_all = test01,
+	.test = test_fanotify,
+	.tcnt = ARRAY_SIZE(tcases),
 	.setup = setup,
 	.cleanup = cleanup,
 	.needs_tmpdir = 1,

Signature · 1 line

2.17.1

Cyril Hrubis

2018-11-21 12:26 UTC #7 lore raw

PATCH 2/5 syscalls/fanotify03: check permission events also on mountpoint mark

Hi!
Pushed, thanks.

Signature · 2 lines

Cyril Hrubis
[email protected]

Amir Goldstein

author

2018-11-16 06:51 UTC #8 lore raw

PATCH 3/5 syscalls/fanotify01: add test for FAN_MARK_FILESYSTEM events

In reply to Amir Goldstein

Signed-off-by Amir Goldstein <[email protected]>

---
 testcases/kernel/syscalls/fanotify/fanotify.h   | 3 +++
 testcases/kernel/syscalls/fanotify/fanotify01.c | 9 +++++++++
 2 files changed, 12 insertions(+)

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

index 5adef54d7..535f1cef2 100644
@@ -57,6 +57,9 @@ static long fanotify_mark(int fd, unsigned int flags, uint64_t mask,
 #ifndef FAN_MARK_INODE
 #define FAN_MARK_INODE		0
 #endif
+#ifndef FAN_MARK_FILESYSTEM
+#define FAN_MARK_FILESYSTEM	0x00000100
+#endif
 
 struct fanotify_mark_type {
 	unsigned int flag;

testcases/kernel/syscalls/fanotify/fanotify01.c +9 -0

index 5dfb67b61..c3d299474 100644
@@ -44,6 +44,10 @@ static struct tcase {
 		"mount mark events",
 		INIT_FANOTIFY_MARK_TYPE(MOUNT),
 	},
+	{
+		"filesystem mark events",
+		INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
+	},
 };
 
 static char fname[BUF_SIZE];

@@ -68,6 +72,11 @@ static void test_fanotify(unsigned int n)
 	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
 			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
 			  AT_FDCWD, fname) < 0) {
+		if (errno == EINVAL && mark->flag == FAN_MARK_FILESYSTEM) {
+			tst_res(TCONF,
+				"FAN_MARK_FILESYSTEM not supported in kernel?");
+			return;
+		}
 		tst_brk(TBROK | TERRNO,
 			"fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS | %s | "
 			"FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, %s) "

Signature · 1 line

2.17.1

Cyril Hrubis

2018-11-21 13:00 UTC #9 lore raw

PATCH 3/5 syscalls/fanotify01: add test for FAN_MARK_FILESYSTEM events

Hi!
Pushed, thanks.

Signature · 2 lines

Cyril Hrubis
[email protected]

Amir Goldstein

author

2018-11-16 06:51 UTC #10 lore raw

PATCH 4/5 syscalls/fanotify03: add test for FAN_MARK_FILESYSTEM permission events

In reply to Amir Goldstein

Signed-off-by Amir Goldstein <[email protected]>

---
 .../kernel/syscalls/fanotify/fanotify03.c     | 25 ++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

testcases/kernel/syscalls/fanotify/fanotify03.c +22 -3

index 19daf57ef..5c105ed32 100644
@@ -46,6 +46,7 @@ static unsigned long long event_set[EVENT_MAX];
 static unsigned int event_resp[EVENT_MAX];
 
 static char event_buf[EVENT_BUF_LEN];
+static int support_perm_events;
 
 static struct tcase {
 	const char *tname;

@@ -59,6 +60,10 @@ static struct tcase {
 		"mount mark permission events",
 		INIT_FANOTIFY_MARK_TYPE(MOUNT),
 	},
+	{
+		"filesystem mark permission events",
+		INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
+	},
 };
 
 static void generate_events(void)

@@ -134,7 +139,7 @@ static void check_child(void)
 		tst_res(TFAIL, "child %s", tst_strstatus(child_ret));
 }
 
-static void setup_mark(unsigned int n)
+static int setup_mark(unsigned int n)
 {
 	struct tcase *tc = &tcases[n];
 	struct fanotify_mark_type *mark = &tc->mark;

@@ -144,7 +149,12 @@ static void setup_mark(unsigned int n)
 	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
 			  FAN_ACCESS_PERM | FAN_OPEN_PERM,
 			  AT_FDCWD, fname) < 0) {
-		if (errno == EINVAL) {
+		if (errno == EINVAL && support_perm_events &&
+		    mark->flag == FAN_MARK_FILESYSTEM) {
+			tst_res(TCONF,
+				"FAN_MARK_FILESYSTEM not supported in kernel?");
+			return -1;
+		} else if (errno == EINVAL) {
 			tst_brk(TCONF | TERRNO,
 				"CONFIG_FANOTIFY_ACCESS_PERMISSIONS not "
 				"configured in kernel?");

@@ -155,9 +165,16 @@ static void setup_mark(unsigned int n)
 				"AT_FDCWD, %s) failed.",
 				fd_notify, mark->name, fname);
 		}
+	} else {
+		/*
+		 * To distigouish between perm event not supported and
+		 * filesystem mark not supported.
+		 */
+		support_perm_events = 1;
 	}
 
 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
+	return 0;
 }
 
 static void test_fanotify(unsigned int n)

@@ -165,7 +182,9 @@ static void test_fanotify(unsigned int n)
 	int tst_count;
 	int ret, len = 0, i = 0, test_num = 0;
 
-	setup_mark(n);
+	if (setup_mark(n) != 0)
+		return;
+
 	run_child();
 
 	tst_count = 0;

Signature · 1 line

2.17.1

Cyril Hrubis

2018-11-21 13:14 UTC #11 lore raw

PATCH 4/5 syscalls/fanotify03: add test for FAN_MARK_FILESYSTEM permission events

Hi!

Quoted 20 lines

testcases/kernel/syscalls/fanotify/fanotify03.c +7 -2

-static void setup_mark(unsigned int n)
+static int setup_mark(unsigned int n)
 {
 	struct tcase *tc = &tcases[n];
 	struct fanotify_mark_type *mark = &tc->mark;

@@ -144,7 +149,12 @@ static void setup_mark(unsigned int n)
 	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
 			  FAN_ACCESS_PERM | FAN_OPEN_PERM,
 			  AT_FDCWD, fname) < 0) {
-		if (errno == EINVAL) {
+		if (errno == EINVAL && support_perm_events &&
+		    mark->flag == FAN_MARK_FILESYSTEM) {
+			tst_res(TCONF,
+				"FAN_MARK_FILESYSTEM not supported in kernel?");
+			return -1;
+		} else if (errno == EINVAL) {
 			tst_brk(TCONF | TERRNO,
 				"CONFIG_FANOTIFY_ACCESS_PERMISSIONS not "
 				"configured in kernel?");

testcases/kernel/syscalls/fanotify/fanotify03.c static void setup_mark(unsigned int n) · line 173

Quoted 9 lines

testcases/kernel/syscalls/fanotify/fanotify03.c +6 -0

@@ -155,9 +165,16 @@ static void setup_mark(unsigned int n)
 				"AT_FDCWD, %s) failed.",
 				fd_notify, mark->name, fname);
 		}
+	} else {
+		/*
+		 * To distigouish between perm event not supported and
+		 * filesystem mark not supported.
+		 */
+		support_perm_events = 1;

I'm a bit puzzled here, so we attempted to cache if perm_events are supported here?

I guess that we depend on the order of the tcases[] array here, which is not very nice.

Also it does not have to be in else branch, if we get EINVAL the first time we call fanotify_mark() tst_brk() is called, which exits the test, so if we ever get to this point in the program, we did at least one successful mark.

Quoted 19 lines

testcases/kernel/syscalls/fanotify/fanotify03.c +4 -1

 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
+	return 0;
 }
 
 static void test_fanotify(unsigned int n)

@@ -165,7 +182,9 @@ static void test_fanotify(unsigned int n)
 	int tst_count;
 	int ret, len = 0, i = 0, test_num = 0;
 
-	setup_mark(n);
+	if (setup_mark(n) != 0)
+		return;
+
 	run_child();
 
 	tst_count = 0;

Signature · 1 line

2.17.1

Signature · 2 lines

Cyril Hrubis
[email protected]

Amir Goldstein

author

2018-11-21 16:36 UTC #12 lore raw

PATCH 4/5 syscalls/fanotify03: add test for FAN_MARK_FILESYSTEM permission events

Quoted 26 lines · Cyril Hrubis

Hi!

Quoted 24 lines

testcases/kernel/syscalls/fanotify/fanotify03.c +8 -2

-static void setup_mark(unsigned int n)
+static int setup_mark(unsigned int n)
 {
      struct tcase *tc = &tcases[n];
      struct fanotify_mark_type *mark = &tc->mark;

@@ -144,7 +149,12 @@ static void setup_mark(unsigned int n)
      if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
                        FAN_ACCESS_PERM | FAN_OPEN_PERM,
                        AT_FDCWD, fname) < 0) {
-             if (errno == EINVAL) {
+             if (errno == EINVAL && support_perm_events &&
+                 mark->flag == FAN_MARK_FILESYSTEM) {
+                     tst_res(TCONF,
+                             "FAN_MARK_FILESYSTEM not supported in kernel?");
+                     return -1;
+             } else if (errno == EINVAL) {
                      tst_brk(TCONF | TERRNO,
                              "CONFIG_FANOTIFY_ACCESS_PERMISSIONS not "
                              "configured in kernel?");

@@ -155,9 +165,16 @@ static void setup_mark(unsigned int n)
                              "AT_FDCWD, %s) failed.",
                              fd_notify, mark->name, fname);
              }
+     } else {

Quoted 9 lines

testcases/kernel/syscalls/fanotify/fanotify03.c static void setup_mark(unsigned int n) · line 173
Quoted 5 lines
testcases/kernel/syscalls/fanotify/fanotify03.c +5 -0
+             /*
+              * To distigouish between perm event not supported and
+              * filesystem mark not supported.
+              */
+             support_perm_events = 1;
I'm a bit puzzled here, so we attempted to cache if perm_events are supported here?

Yes.

Quoted 3 lines

I guess that we depend on the order of the tcases[] array here, which is not very nice.

Indeed. It is a simplification. All test cases are permission events, so the order of failure types is not likely to change, but I could add a comment about the order near test cases definition.
Would you rather that I did an explicit test for permission support at setup() time?

Quoted 4 lines

Also it does not have to be in else branch, if we get EINVAL the first time we call fanotify_mark() tst_brk() is called, which exits the test, so if we ever get to this point in the program, we did at least one successful mark.

Right. I'll change that.

Thanks,
Amir.

Amir Goldstein

author

2018-11-21 17:10 UTC #13 lore raw

PATCH 4/5 syscalls/fanotify03: add test for FAN_MARK_FILESYSTEM permission events

Quoted 44 lines · Amir Goldstein

Quoted 26 lines · Cyril Hrubis

Hi!

Quoted 24 lines

testcases/kernel/syscalls/fanotify/fanotify03.c +8 -2

-static void setup_mark(unsigned int n)
+static int setup_mark(unsigned int n)
 {
      struct tcase *tc = &tcases[n];
      struct fanotify_mark_type *mark = &tc->mark;

@@ -144,7 +149,12 @@ static void setup_mark(unsigned int n)
      if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
                        FAN_ACCESS_PERM | FAN_OPEN_PERM,
                        AT_FDCWD, fname) < 0) {
-             if (errno == EINVAL) {
+             if (errno == EINVAL && support_perm_events &&
+                 mark->flag == FAN_MARK_FILESYSTEM) {
+                     tst_res(TCONF,
+                             "FAN_MARK_FILESYSTEM not supported in kernel?");
+                     return -1;
+             } else if (errno == EINVAL) {
                      tst_brk(TCONF | TERRNO,
                              "CONFIG_FANOTIFY_ACCESS_PERMISSIONS not "
                              "configured in kernel?");

@@ -155,9 +165,16 @@ static void setup_mark(unsigned int n)
                              "AT_FDCWD, %s) failed.",
                              fd_notify, mark->name, fname);
              }
+     } else {

Quoted 9 lines

testcases/kernel/syscalls/fanotify/fanotify03.c static void setup_mark(unsigned int n) · line 173
Quoted 5 lines
testcases/kernel/syscalls/fanotify/fanotify03.c +5 -0
+             /*
+              * To distigouish between perm event not supported and
+              * filesystem mark not supported.
+              */
+             support_perm_events = 1;
I'm a bit puzzled here, so we attempted to cache if perm_events are supported here?

Yes.

Quoted 3 lines

I guess that we depend on the order of the tcases[] array here, which is not very nice.

Quoted 6 lines

Indeed. It is a simplification. All test cases are permission events, so the order of failure types is not likely to change, but I could add a comment about the order near test cases definition.
Would you rather that I did an explicit test for permission support at setup() time?

Nevermind. this discussion is moot because no permission events support breaks out of the test. I'll drop this logic altogether.

It will have to make a comeback though with Mark's patches to add tests for FAN_OPEN_EXEC_PERM. (need to distinguish between to support for FAN_MARK_FILESYSTEM and no support for FAN_OPEN_EXEC_PERM)
so please state your preference for the best way to handle this issue.

Thanks,
Amir.

Cyril Hrubis

2018-11-22 16:03 UTC #14 lore raw

PATCH 4/5 syscalls/fanotify03: add test for FAN_MARK_FILESYSTEM permission events

In reply to Amir Goldstein

Hi!

Quoted 9 lines

Quoted 3 lines
I guess that we depend on the order of the tcases[] array here, which is not very nice.
Indeed. It is a simplification. All test cases are permission events, so the order of failure types is not likely to change, but I could add a comment about the order near test cases definition.
Would you rather that I did an explicit test for permission support at setup() time?

Either one would be fine.

Signature · 2 lines

Cyril Hrubis
[email protected]

Amir Goldstein

author

2018-11-16 06:51 UTC #15 lore raw

PATCH 5/5 syscalls/fanotify10: add tests for FAN_MARK_FILESYSTEM and ignore mask

In reply to Amir Goldstein

A watch with FAN_MARK_FILESYSTEM should ignore events that were generated on files whose inode have an ignore mask and ignore events generated on a mount that has a mark with ignore mask.

Signed-off-by Amir Goldstein <[email protected]>

---
 .../kernel/syscalls/fanotify/fanotify10.c     | 40 ++++++++++++++++++-
 1 file changed, 38 insertions(+), 2 deletions(-)

testcases/kernel/syscalls/fanotify/fanotify10.c +38 -2

index 2b96de2f1..820c50a8a 100644
@@ -66,11 +66,13 @@ static int bind_mount_created;
 enum {
 	FANOTIFY_INODE,
 	FANOTIFY_MOUNT,
+	FANOTIFY_FILESYSTEM,
 };
 
 static struct fanotify_mark_type fanotify_mark_types[] = {
 	INIT_FANOTIFY_MARK_TYPE(INODE),
 	INIT_FANOTIFY_MARK_TYPE(MOUNT),
+	INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
 };
 
 static struct tcase {

@@ -106,9 +108,33 @@ static struct tcase {
 		MNT2_PATH, FANOTIFY_MOUNT,
 		FILE_PATH, 1
 	},
+	{
+		"ignore fs events created on a specific file",
+		MOUNT_PATH, FANOTIFY_FILESYSTEM,
+		FILE_PATH, FANOTIFY_INODE,
+		FILE_PATH, 0
+	},
+	{
+		"don't ignore mount events created on another file",
+		MOUNT_PATH, FANOTIFY_FILESYSTEM,
+		FILE_PATH, FANOTIFY_INODE,
+		FILE2_PATH, 1
+	},
+	{
+		"ignore fs events created on a specific mount point",
+		MOUNT_PATH, FANOTIFY_FILESYSTEM,
+		MNT2_PATH, FANOTIFY_MOUNT,
+		FILE_MNT2, 0
+	},
+	{
+		"don't ignore fs events created on another mount point",
+		MOUNT_PATH, FANOTIFY_FILESYSTEM,
+		MNT2_PATH, FANOTIFY_MOUNT,
+		FILE_PATH, 1
+	},
 };
 
-static void create_fanotify_groups(unsigned int n)
+static int create_fanotify_groups(unsigned int n)
 {
 	struct tcase *tc = &tcases[n];
 	struct fanotify_mark_type *mark, *ignore_mark;

@@ -129,6 +155,13 @@ static void create_fanotify_groups(unsigned int n)
 					    FAN_MARK_ADD | mark->flag,
 					    FAN_OPEN, AT_FDCWD, tc->mark_path);
 			if (ret < 0) {
+				if (errno == EINVAL &&
+				    tc->mark_type == FANOTIFY_FILESYSTEM) {
+					tst_res(TCONF,
+						"FAN_MARK_FILESYSTEM not "
+						"supported in kernel?");
+					return -1;
+				}
 				tst_brk(TBROK | TERRNO,
 					"fanotify_mark(%d, FAN_MARK_ADD | %s,"
 					"FAN_OPEN, AT_FDCWD, %s) failed",

@@ -155,6 +188,7 @@ static void create_fanotify_groups(unsigned int n)
 			}
 		}
 	}
+	return 0;
 }
 
 static void cleanup_fanotify_groups(void)

@@ -198,7 +232,8 @@ static void test_fanotify(unsigned int n)
 
 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
 
-	create_fanotify_groups(n);
+	if (create_fanotify_groups(n) != 0)
+		goto cleanup;
 
 	mark = &fanotify_mark_types[tc->mark_type];
 	ignore_mark = &fanotify_mark_types[tc->ignore_mark_type];

@@ -268,6 +303,7 @@ static void test_fanotify(unsigned int n)
 			}
 		}
 	}
+cleanup:
 	cleanup_fanotify_groups();
 }

Signature · 1 line

2.17.1

Cyril Hrubis

2018-11-21 13:27 UTC #16 lore raw

PATCH 5/5 syscalls/fanotify10: add tests for FAN_MARK_FILESYSTEM and ignore mask

Hi!
Pushed, thanks.

Signature · 2 lines

Cyril Hrubis
[email protected]