diff mbox series

[5/9] syscalls/fanotify10: Add more verifications and debug info

Message ID 20220905154239.2652169-6-amir73il@gmail.com
State Accepted
Headers show
Series Fanotify tests for FAN_MARK_IGNORE | expand

Commit Message

Amir Goldstein Sept. 5, 2022, 3:42 p.m. UTC 
Check that non-evictable inode ignore marks exist as expected
and print mask of unexpected events.

Fix information printed for events from unexpected pid.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 testcases/kernel/syscalls/fanotify/fanotify.h |  4 ++
 .../kernel/syscalls/fanotify/fanotify10.c     | 41 ++++++++++++-------
 2 files changed, 31 insertions(+), 14 deletions(-)

Comments

Petr Vorel Sept. 6, 2022, 5:01 p.m. UTC | #1 
Hi Amir, all,

> Check that non-evictable inode ignore marks exist as expected
> and print mask of unexpected events.

> Fix information printed for events from unexpected pid.

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

...
> +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
...
> @@ -582,14 +591,18 @@ static void test_fanotify(unsigned int n)
>  	for (p = 1; p < num_classes && !tc->expected_mask_with_ignore; p++) {
>  		for (i = 0; i < GROUPS_PER_PRIO; i++) {
>  			ret = read(fd_notify[p][i], event_buf, EVENT_BUF_LEN);
> -			if (ret == 0) {
> +			if (ret >= 0 && ret < (int)FAN_EVENT_METADATA_LEN) {
>  				tst_brk(TBROK,
> -					"zero length read from fanotify fd");
> +					"short read when reading fanotify "
> +					"events (%d < %d)", ret,
> +					(int)EVENT_BUF_LEN);
Just for the record printing like this...

>  			}
> +			event = (struct fanotify_event_metadata *)event_buf;
>  			if (ret > 0) {
>  				tst_res(TFAIL, "group %d (%x) with %s and "
> -					"%s ignore mask got event",
> -					i, fanotify_class[p], mark->name, ignore_mark->name);
> +					"%s ignore mask got unexpected event (mask %llx)",
> +					i, fanotify_class[p], mark->name, ignore_mark->name,
> +					event->mask);
and this will be possible to avoid with macros include/tst_test_macros.h,
which prints them for free. FYI there can be also custom error text instead
of the failing syscall with parameters printed by default. But IMHO there should
be a reason to use it (mostly the default should be good enough, in case of
failure test reviewers will have to look into the test source anyway).

Kind regards,
Petr

>  				if (event->fd != FAN_NOFD)
>  					SAFE_CLOSE(event->fd);
>  			} else if (errno == EAGAIN) {
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index d67c079af..810a48e81 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -112,6 +112,10 @@  static inline int safe_fanotify_mark(const char *file, const int lineno,
 #ifndef FAN_MARK_IGNORE_SURV
 #define FAN_MARK_IGNORE_SURV	(FAN_MARK_IGNORE | FAN_MARK_IGNORED_SURV_MODIFY)
 #endif
+/* Non-uapi convenience macros */
+#ifndef FAN_MARK_TYPES
+#define FAN_MARK_TYPES (FAN_MARK_INODE | FAN_MARK_MOUNT | FAN_MARK_FILESYSTEM)
+#endif
 
 /* New dirent event masks */
 #ifndef FAN_ATTRIB
diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index b8358b489..ea05e4ff0 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -328,18 +328,21 @@  static struct tcase {
 	},
 };
 
-static void show_fanotify_marks(int fd)
+static void show_fanotify_ignore_marks(int fd, int expected)
 {
 	unsigned int mflags, mask, ignored_mask;
 	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:%x ignored_mask:%x",
-				&mflags, &mask, &ignored_mask)) {
-		tst_res(TPASS, "No fanotify inode marks as expected");
+				&mflags, &mask, &ignored_mask) || !ignored_mask) {
+		tst_res(!expected ? TPASS : TFAIL,
+			"No fanotify inode ignore marks %sexpected",
+			!expected ? "as " : "is un");
 	} else {
-		tst_res(TFAIL, "Unexpected inode mark (mflags=%x, mask=%x ignored_mask=%x)",
-				mflags, mask, ignored_mask);
+		tst_res(expected ? TPASS : TFAIL,
+			"Found %sexpected inode ignore mark (mflags=%x, mask=%x ignored_mask=%x)",
+			expected ? "" : "un", mflags, mask, ignored_mask);
 	}
 }
 
@@ -358,9 +361,11 @@  static int create_fanotify_groups(unsigned int n)
 	unsigned int mark_ignored, mask;
 	unsigned int p, i;
 	int evictable_ignored = (tc->ignore_mark_type == FANOTIFY_EVICTABLE);
+	int ignore_mark_type;
 
 	mark = &fanotify_mark_types[tc->mark_type];
 	ignore_mark = &fanotify_mark_types[tc->ignore_mark_type];
+	ignore_mark_type = ignore_mark->flag & FAN_MARK_TYPES;
 
 	/* Open fd for syncfs before creating groups to avoid the FAN_OPEN event */
 	fd_syncfs = SAFE_OPEN(MOUNT_PATH, O_RDONLY);
@@ -382,7 +387,7 @@  static int create_fanotify_groups(unsigned int n)
 					    FAN_EVENT_ON_CHILD,
 					    AT_FDCWD, tc->mark_path);
 
-			/* Add ignore mark for groups with higher priority */
+			/* Do not add ignore mark for first priority groups */
 			if (p == 0)
 				continue;
 
@@ -411,14 +416,18 @@  add_mark:
 	}
 
 	/*
-	 * drop_caches should evict inode from cache and remove evictable marks
+	 * Verify that first priority groups have no ignore inode marks and that
+	 * drop_caches evicted the evictable ignore marks of other groups.
 	 */
-	if (evictable_ignored) {
+	if (evictable_ignored)
 		drop_caches();
+
+	if (ignore_mark_type == FAN_MARK_INODE) {
 		for (p = 0; p < num_classes; p++) {
 			for (i = 0; i < GROUPS_PER_PRIO; i++) {
 				if (fd_notify[p][i] > 0)
-					show_fanotify_marks(fd_notify[p][i]);
+					show_fanotify_ignore_marks(fd_notify[p][i],
+								   p > 0 && !evictable_ignored);
 			}
 		}
 	}
@@ -464,7 +473,7 @@  static void verify_event(int p, int group, struct fanotify_event_metadata *event
 		tst_res(TFAIL, "group %d (%x) got event: mask %llx pid=%u "
 			"(expected %u) fd=%u", group, fanotify_class[p],
 			(unsigned long long)event->mask, (unsigned int)event->pid,
-			(unsigned int)getpid(), event->fd);
+			(unsigned int)child_pid, event->fd);
 	} else {
 		tst_res(TPASS, "group %d (%x) got event: mask %llx pid=%u fd=%u",
 			group, fanotify_class[p], (unsigned long long)event->mask,
@@ -582,14 +591,18 @@  static void test_fanotify(unsigned int n)
 	for (p = 1; p < num_classes && !tc->expected_mask_with_ignore; p++) {
 		for (i = 0; i < GROUPS_PER_PRIO; i++) {
 			ret = read(fd_notify[p][i], event_buf, EVENT_BUF_LEN);
-			if (ret == 0) {
+			if (ret >= 0 && ret < (int)FAN_EVENT_METADATA_LEN) {
 				tst_brk(TBROK,
-					"zero length read from fanotify fd");
+					"short read when reading fanotify "
+					"events (%d < %d)", ret,
+					(int)EVENT_BUF_LEN);
 			}
+			event = (struct fanotify_event_metadata *)event_buf;
 			if (ret > 0) {
 				tst_res(TFAIL, "group %d (%x) with %s and "
-					"%s ignore mask got event",
-					i, fanotify_class[p], mark->name, ignore_mark->name);
+					"%s ignore mask got unexpected event (mask %llx)",
+					i, fanotify_class[p], mark->name, ignore_mark->name,
+					event->mask);
 				if (event->fd != FAN_NOFD)
 					SAFE_CLOSE(event->fd);
 			} else if (errno == EAGAIN) {