Skip to contentSkip to conversation

syscalls/fanotify03: defined additional tcase members for added control

PATCH New Matthew Bobrowski 3 participants 6 messages Jan 22, 2019 – Feb 26, 2019

lore
Share & tools
lore
#0 lore raw

PATCH 1/5 syscalls/fanotify03: defined additional tcase members for added control

* Included the event mask member to be used for fanotify_mark() within the tcase. This allows control over what event types are to be generated on a tcase by tcase level as oppose to having it globally defined for all tcases.

* Included the event_set and event_resp arrays into a tcase so that the sequence of expected events can be defined on a test case by test case level. Both event_set and event_resp are now represented by the event struct.

* Added event_count member to tcase in order to represent the number of expected events for each tcase.

* Fanotify permission events cannot be merged, thus cleaned up some unnecessary code in test_fanotify() that checks whether any additional bits are left within event->mask.

---
 testcases/kernel/syscalls/fanotify/fanotify03.c | 96 ++++++++++++++-----------
 1 file changed, 56 insertions(+), 40 deletions(-)
testcases/kernel/syscalls/fanotify/fanotify03.c +56 -40
index 0bf64577f..aa16039a3 100644
@@ -32,6 +32,8 @@
 #define EVENT_SIZE  (sizeof (struct fanotify_event_metadata))
 /* reasonable guess as to size of 1024 events */
 #define EVENT_BUF_LEN        (EVENT_MAX * EVENT_SIZE)
+/* Size large enough to hold a reasonable amount of expected event objects */
+#define EVENT_SET_MAX 16
 
 #define BUF_SIZE 256
 #define TST_TOTAL 3
@@ -42,28 +44,48 @@ static volatile int fd_notify;
 
 static pid_t child_pid;
 
-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;
 
+struct event {
+	unsigned long long mask;
+	unsigned int response;
+};
+
 static struct tcase {
 	const char *tname;
 	struct fanotify_mark_type mark;
+	unsigned long long mask;
+	int event_count;
+	struct event event_set[EVENT_SET_MAX];
 } tcases[] = {
 	{
 		"inode mark permission events",
 		INIT_FANOTIFY_MARK_TYPE(INODE),
+		FAN_OPEN_PERM | FAN_ACCESS_PERM, 2,
+		{
+			{FAN_OPEN_PERM, FAN_ALLOW},
+			{FAN_ACCESS_PERM, FAN_DENY}
+		}
 	},
 	{
 		"mount mark permission events",
 		INIT_FANOTIFY_MARK_TYPE(MOUNT),
+		FAN_OPEN_PERM | FAN_ACCESS_PERM, 2,
+		{
+			{FAN_OPEN_PERM, FAN_ALLOW},
+			{FAN_ACCESS_PERM, FAN_DENY}
+		}
 	},
 	{
 		"filesystem mark permission events",
 		INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
-	},
+		FAN_OPEN_PERM | FAN_ACCESS_PERM, 2,
+		{
+			{FAN_OPEN_PERM, FAN_ALLOW},
+			{FAN_ACCESS_PERM, FAN_DENY}
+		}
+	}
 };
 
 static void generate_events(void)
@@ -146,8 +168,7 @@ static int setup_mark(unsigned int n)
 
 	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
 
-	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
-			  FAN_ACCESS_PERM | FAN_OPEN_PERM,
+	if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
 			  AT_FDCWD, fname) < 0) {
 		if (errno == EINVAL && mark->flag == FAN_MARK_FILESYSTEM) {
 			tst_res(TCONF,
@@ -166,7 +187,7 @@ static int setup_mark(unsigned int n)
 		}
 	} else {
 		/*
-		 * To distigouish between perm event not supported and
+		 * To distinguish between perm event not supported and
 		 * filesystem mark not supported.
 		 */
 		support_perm_events = 1;
@@ -178,32 +199,22 @@ static int setup_mark(unsigned int n)
 
 static void test_fanotify(unsigned int n)
 {
-	int tst_count;
 	int ret, len = 0, i = 0, test_num = 0;
+	struct tcase *tc = &tcases[n];
+	struct event *event_set = tc->event_set;
 
 	if (setup_mark(n) != 0)
 		return;
 
 	run_child();
 
-	tst_count = 0;
-
-	event_set[tst_count] = FAN_OPEN_PERM;
-	event_resp[tst_count++] = FAN_ALLOW;
-	event_set[tst_count] = FAN_ACCESS_PERM;
-	event_resp[tst_count++] = FAN_DENY;
-
-	/* tst_count + 1 is for checking child return value */
-	if (TST_TOTAL != tst_count + 1) {
-		tst_brk(TBROK,
-			"TST_TOTAL and tst_count do not match");
-	}
-	tst_count = 0;
-
 	/*
-	 * check events
+	 * Process events
+	 *
+	 * tc->count + 1 is to accommodate for checking the child process
+	 * return value
 	 */
-	while (test_num < TST_TOTAL && fd_notify != -1) {
+	while (test_num < tc->event_count + 1 && fd_notify != -1) {
 		struct fanotify_event_metadata *event;
 
 		if (i == len) {
@@ -221,12 +232,16 @@ static void test_fanotify(unsigned int n)
 		}
 
 		event = (struct fanotify_event_metadata *)&event_buf[i];
-		if (!(event->mask & event_set[test_num])) {
+		/* Permission events cannot be merged, so the event mask
+		 * reported should exactly match the event mask within the
+		 * event set.
+		 */
+		if (event->mask != event_set[test_num].mask) {
 			tst_res(TFAIL,
 				"got event: mask=%llx (expected %llx) "
 				"pid=%u fd=%d",
 				(unsigned long long)event->mask,
-				event_set[test_num],
+				event_set[test_num].mask,
 				(unsigned)event->pid, event->fd);
 		} else if (event->pid != child_pid) {
 			tst_res(TFAIL,
@@ -242,29 +257,30 @@ static void test_fanotify(unsigned int n)
 				(unsigned long long)event->mask,
 				(unsigned)event->pid, event->fd);
 		}
-		/* Write response to permission event */
-		if (event_set[test_num] & FAN_ALL_PERM_EVENTS) {
+
+		/* Write response to the permission event */
+		if (event_set[test_num].mask & FAN_ALL_PERM_EVENTS) {
 			struct fanotify_response resp;
 
 			resp.fd = event->fd;
-			resp.response = event_resp[test_num];
-			SAFE_WRITE(1, fd_notify, &resp,
-				   sizeof(resp));
-		}
-		event->mask &= ~event_set[test_num];
-		/* No events left in current mask? Go for next event */
-		if (event->mask == 0) {
-			i += event->event_len;
-			if (event->fd != FAN_NOFD)
-				SAFE_CLOSE(event->fd);
+			resp.response = event_set[test_num].response;
+			SAFE_WRITE(1, fd_notify, &resp, sizeof(resp));
 		}
+
+		i += event->event_len;
+
+		if (event->fd != FAN_NOFD)
+			SAFE_CLOSE(event->fd);
+
 		test_num++;
 	}
-	for (; test_num < TST_TOTAL - 1; test_num++) {
+
+	for (; test_num < tc->event_count; test_num++) {
 		tst_res(TFAIL, "didn't get event: mask=%llx",
-			event_set[test_num]);
+			event_set[test_num].mask);
 
 	}
+
 	check_child();
 
 	if (fd_notify > 0)
Signature · 5 lines
2.16.4


-- 
Matthew Bobrowski
#1 lore raw

PATCH 1/5 syscalls/fanotify03: defined additional tcase members for added control

Hi!
I've finally started to look into this patchset (sorry for the long delay), the changes to fanotify03 looks good to me and I wanted to push them, but I'm getting rejects:

Checking patch testcases/kernel/syscalls/fanotify/fanotify03.c...
error: while searching for:

static pid_t child_pid;

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;
        struct fanotify_mark_type mark;
} tcases[] = {
        {
                "inode mark permission events",
                INIT_FANOTIFY_MARK_TYPE(INODE),
        },
        {
                "mount mark permission events",
                INIT_FANOTIFY_MARK_TYPE(MOUNT),
        },
        {
                "filesystem mark permission events",
                INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
        },
};

static void generate_events(void)

error: patch failed: testcases/kernel/syscalls/fanotify/fanotify03.c:42 Hunk #3 succeeded at 147 (offset -21 lines).
error: while searching for:

                }
        } else {
                /*
                 * To distigouish between perm event not supported and
                 * filesystem mark not supported.
                 */
                support_perm_events = 1;

Is this based on some yet to be merged patchset?

Signature · 2 lines
Cyril Hrubis
[email protected]
#2 lore raw

PATCH 1/5 syscalls/fanotify03: defined additional tcase members for added control

Quoted 49 lines · Cyril Hrubis

Hi!
I've finally started to look into this patchset (sorry for the long delay), the changes to fanotify03 looks good to me and I wanted to push them, but I'm getting rejects:

Checking patch testcases/kernel/syscalls/fanotify/fanotify03.c...
error: while searching for:

static pid_t child_pid;

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;
        struct fanotify_mark_type mark;
} tcases[] = {
        {
                "inode mark permission events",
                INIT_FANOTIFY_MARK_TYPE(INODE),
        },
        {
                "mount mark permission events",
                INIT_FANOTIFY_MARK_TYPE(MOUNT),
        },
        {
                "filesystem mark permission events",
                INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
        },
};

static void generate_events(void)

error: patch failed: testcases/kernel/syscalls/fanotify/fanotify03.c:42 Hunk #3 succeeded at 147 (offset -21 lines).
error: while searching for:

                }
        } else {
                /*
                 * To distigouish between perm event not supported and
                 * filesystem mark not supported.
                 */
                support_perm_events = 1;
Quoted 1 line

Is this based on some yet to be merged patchset?

I see the problem.
It is based on an older version of
syscalls/fanotify03: add test for FAN_MARK_FILESYSTEM permission events

You removed support_perm_events on commit or maybe I sent another version based on your review - not sure.

Attached the missing prerequisite patch.

You may choose to drop support_perm_events again or add a test for kernel permission events support in setup(), but we do need to distinguish between support_exec_events (which comes in a later patch) and FAN_MARK_FILESYSTEM support, as exec events may be backported to distro stable kernels and FAN_MARK_FILESYSTEM is less likely to be backported.

Thanks,
Amir.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-syscalls-fanotify03-add-test-for-FAN_MARK_FILESYSTEM.patch
Type: text/x-patch
Size: 1218 bytes
Desc: not available
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190222/ac8e5842/attachment.bin>

#3 lore raw

PATCH 1/5 syscalls/fanotify03: defined additional tcase members for added control

Hi!

Quoted 7 lines

I see the problem.
It is based on an older version of
syscalls/fanotify03: add test for FAN_MARK_FILESYSTEM permission events

You removed support_perm_events on commit or maybe I sent another version based on your review - not sure.

Quoted 8 lines

Attached the missing prerequisite patch.

You may choose to drop support_perm_events again or add a test for kernel permission events support in setup(), but we do need to distinguish between support_exec_events (which comes in a later patch) and FAN_MARK_FILESYSTEM support, as exec events may be backported to distro stable kernels and FAN_MARK_FILESYSTEM is less likely to be backported.

Unfortunately that did the trick only for the first patch, the rest is still broken :-(.

Signature · 2 lines
Cyril Hrubis
[email protected]
#4 lore raw

PATCH 1/5 syscalls/fanotify03: defined additional tcase members for added control

Quoted 18 lines · Cyril Hrubis

Hi!

Quoted 7 lines

I see the problem.
It is based on an older version of
syscalls/fanotify03: add test for FAN_MARK_FILESYSTEM permission events

You removed support_perm_events on commit or maybe I sent another version based on your review - not sure.

Quoted 8 lines

Attached the missing prerequisite patch.

You may choose to drop support_perm_events again or add a test for kernel permission events support in setup(), but we do need to distinguish between support_exec_events (which comes in a later patch) and FAN_MARK_FILESYSTEM support, as exec events may be backported to distro stable kernels and FAN_MARK_FILESYSTEM is less likely to be backported.

Quoted 3 lines

Unfortunately that did the trick only for the first patch, the rest is still broken :-(.

Here is my branch with Matthew's patches, rebased on master and tested:
https://github.com/amir73il/ltp/commits/fanotify_exec

Thanks,
Amir.