[LTP] [PATCH 0/2] Fanotify tests for v5.19-rc5

[LTP] [PATCH 0/2] Fanotify tests for v5.19-rc5

[Amir Goldstein]

Hi Petr,

This updates the tests to validate the current ENOTDIR behavior of
fanotify that was merged for v5.19-rc5 and v5.18.10.

This is a retroactive change for APIs FAN_REPORT_TARGET_FID
and FAN_RENAME introduced in v5.17.

The current upstream behavior is already documented in man pages [1].

Thanks,
Amir.

[1] https://lore.kernel.org/linux-man/20220630190610.3043428-1-amir73il@gmail.com/

Amir Goldstein (2):
  syscalls/fanotify14: Encode the expected errno in test case
  syscalls/fanotify14: Add test cases for events not allowed on non-dir

 testcases/kernel/syscalls/fanotify/fanotify.h |   4 +-
 .../kernel/syscalls/fanotify/fanotify14.c     | 102 +++++++++++++++---
 2 files changed, 88 insertions(+), 18 deletions(-)

-- 
2.25.1


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

[LTP] [PATCH 1/2] syscalls/fanotify14: Encode the expected errno in test case

[Amir Goldstein]

So we can add test cases for errors other than EINVAL.

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

diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
index 5d74b9b91..c99e19706 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify14.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
@@ -41,38 +41,39 @@ static struct test_case_t {
 	unsigned int init_flags;
 	unsigned int mark_flags;
 	unsigned long long mask;
+	int expected_errno;
 } test_cases[] = {
 	{
-		FAN_CLASS_CONTENT | FAN_REPORT_FID, 0, 0
+		FAN_CLASS_CONTENT | FAN_REPORT_FID, 0, 0, EINVAL
 	},
 	{
-		FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID, 0, 0
+		FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID, 0, 0, EINVAL
 	},
 	{
-		FAN_CLASS_NOTIF, 0, INODE_EVENTS
+		FAN_CLASS_NOTIF, 0, INODE_EVENTS, EINVAL
 	},
 	{
-		FAN_CLASS_NOTIF | FAN_REPORT_FID, FAN_MARK_MOUNT, INODE_EVENTS
+		FAN_CLASS_NOTIF | FAN_REPORT_FID, FAN_MARK_MOUNT, INODE_EVENTS, EINVAL
 	},
 	{
 		/* FAN_REPORT_NAME without FAN_REPORT_DIR_FID is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_NAME, 0, 0
+		FAN_CLASS_NOTIF | FAN_REPORT_NAME, 0, 0, EINVAL
 	},
 	{
 		/* FAN_REPORT_NAME without FAN_REPORT_DIR_FID is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_FID | FAN_REPORT_NAME, 0, 0
+		FAN_CLASS_NOTIF | FAN_REPORT_FID | FAN_REPORT_NAME, 0, 0, EINVAL
 	},
 	{
 		/* FAN_REPORT_TARGET_FID without FAN_REPORT_FID is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_NAME, 0, 0
+		FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_NAME, 0, 0, EINVAL
 	},
 	{
 		/* FAN_REPORT_TARGET_FID without FAN_REPORT_NAME is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_FID, 0, 0
+		FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_FID, 0, 0, EINVAL
 	},
 	{
 		/* FAN_RENAME without FAN_REPORT_NAME is not valid */
-		FAN_CLASS_NOTIF | FAN_REPORT_DFID_FID, 0, FAN_RENAME
+		FAN_CLASS_NOTIF | FAN_REPORT_DFID_FID, 0, FAN_RENAME, EINVAL
 	},
 };
 
@@ -88,12 +89,12 @@ static void do_test(unsigned int number)
 		 * an invalid notification class is specified in
 		 * conjunction with FAN_REPORT_FID.
 		 */
-		if (errno == EINVAL) {
+		if (errno == tc->expected_errno) {
 			tst_res(TPASS,
 				"fanotify_fd=%d, fanotify_init(%x, O_RDONLY) "
-				"failed with error EINVAL as expected",
+				"failed with error %d as expected",
 				fanotify_fd,
-				tc->init_flags);
+				tc->init_flags, tc->expected_errno);
 			return;
 		}
 		tst_brk(TBROK | TERRNO,
@@ -126,16 +127,16 @@ static void do_test(unsigned int number)
 		 * specified on the notification group, or using
 		 * INODE_EVENTS with mark type FAN_MARK_MOUNT.
 		 */
-		if (errno == EINVAL) {
+		if (errno == tc->expected_errno) {
 			tst_res(TPASS,
 				"ret=%d, fanotify_mark(%d, FAN_MARK_ADD | %x, "
-				"%llx, AT_FDCWD, %s) failed with error EINVAL "
+				"%llx, AT_FDCWD, %s) failed with error %d "
 				"as expected",
 				ret,
 				fanotify_fd,
 				tc->mark_flags,
 				tc->mask,
-				FILE1);
+				FILE1, tc->expected_errno);
 			goto out;
 		}
 		tst_brk(TBROK | TERRNO,
-- 
2.25.1


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

Re: [LTP] [PATCH 1/2] syscalls/fanotify14: Encode the expected errno in test case

[Matthew Bobrowski via ltp]

On Tue, Jul 19, 2022 at 11:58:52AM +0200, Amir Goldstein wrote:
> So we can add test cases for errors other than EINVAL.

Just one optional nit. We can probably remove the comments which are
directly above the existing `if (errno == EINVAL)` checks as they're
specific to one expected errno value, but this is no longer the case
with ENOTDIR now in some fanotify_init/fanotify_mark cases.

Feel free to add RVB tags.

> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  .../kernel/syscalls/fanotify/fanotify14.c     | 31 ++++++++++---------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
> index 5d74b9b91..c99e19706 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify14.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
> @@ -41,38 +41,39 @@ static struct test_case_t {
>  	unsigned int init_flags;
>  	unsigned int mark_flags;
>  	unsigned long long mask;
> +	int expected_errno;
>  } test_cases[] = {
>  	{
> -		FAN_CLASS_CONTENT | FAN_REPORT_FID, 0, 0
> +		FAN_CLASS_CONTENT | FAN_REPORT_FID, 0, 0, EINVAL
>  	},
>  	{
> -		FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID, 0, 0
> +		FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID, 0, 0, EINVAL
>  	},
>  	{
> -		FAN_CLASS_NOTIF, 0, INODE_EVENTS
> +		FAN_CLASS_NOTIF, 0, INODE_EVENTS, EINVAL
>  	},
>  	{
> -		FAN_CLASS_NOTIF | FAN_REPORT_FID, FAN_MARK_MOUNT, INODE_EVENTS
> +		FAN_CLASS_NOTIF | FAN_REPORT_FID, FAN_MARK_MOUNT, INODE_EVENTS, EINVAL
>  	},
>  	{
>  		/* FAN_REPORT_NAME without FAN_REPORT_DIR_FID is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_NAME, 0, 0
> +		FAN_CLASS_NOTIF | FAN_REPORT_NAME, 0, 0, EINVAL
>  	},
>  	{
>  		/* FAN_REPORT_NAME without FAN_REPORT_DIR_FID is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_FID | FAN_REPORT_NAME, 0, 0
> +		FAN_CLASS_NOTIF | FAN_REPORT_FID | FAN_REPORT_NAME, 0, 0, EINVAL
>  	},
>  	{
>  		/* FAN_REPORT_TARGET_FID without FAN_REPORT_FID is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_NAME, 0, 0
> +		FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_NAME, 0, 0, EINVAL
>  	},
>  	{
>  		/* FAN_REPORT_TARGET_FID without FAN_REPORT_NAME is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_FID, 0, 0
> +		FAN_CLASS_NOTIF | FAN_REPORT_TARGET_FID | FAN_REPORT_DFID_FID, 0, 0, EINVAL
>  	},
>  	{
>  		/* FAN_RENAME without FAN_REPORT_NAME is not valid */
> -		FAN_CLASS_NOTIF | FAN_REPORT_DFID_FID, 0, FAN_RENAME
> +		FAN_CLASS_NOTIF | FAN_REPORT_DFID_FID, 0, FAN_RENAME, EINVAL
>  	},
>  };
>  
> @@ -88,12 +89,12 @@ static void do_test(unsigned int number)
>  		 * an invalid notification class is specified in
>  		 * conjunction with FAN_REPORT_FID.
>  		 */
> -		if (errno == EINVAL) {
> +		if (errno == tc->expected_errno) {
>  			tst_res(TPASS,
>  				"fanotify_fd=%d, fanotify_init(%x, O_RDONLY) "
> -				"failed with error EINVAL as expected",
> +				"failed with error %d as expected",
>  				fanotify_fd,
> -				tc->init_flags);
> +				tc->init_flags, tc->expected_errno);
>  			return;
>  		}
>  		tst_brk(TBROK | TERRNO,
> @@ -126,16 +127,16 @@ static void do_test(unsigned int number)
>  		 * specified on the notification group, or using
>  		 * INODE_EVENTS with mark type FAN_MARK_MOUNT.
>  		 */
> -		if (errno == EINVAL) {
> +		if (errno == tc->expected_errno) {
>  			tst_res(TPASS,
>  				"ret=%d, fanotify_mark(%d, FAN_MARK_ADD | %x, "
> -				"%llx, AT_FDCWD, %s) failed with error EINVAL "
> +				"%llx, AT_FDCWD, %s) failed with error %d "
>  				"as expected",
>  				ret,
>  				fanotify_fd,
>  				tc->mark_flags,
>  				tc->mask,
> -				FILE1);
> +				FILE1, tc->expected_errno);
>  			goto out;
>  		}
>  		tst_brk(TBROK | TERRNO,
> -- 
> 2.25.1
> 
/M

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

Re: [LTP] [PATCH 1/2] syscalls/fanotify14: Encode the expected errno in test case

[Amir Goldstein]

On Tue, Jul 19, 2022 at 11:58 PM Matthew Bobrowski <repnop@google.com> wrote:
>
> On Tue, Jul 19, 2022 at 11:58:52AM +0200, Amir Goldstein wrote:
> > So we can add test cases for errors other than EINVAL.
>
> Just one optional nit. We can probably remove the comments which are
> directly above the existing `if (errno == EINVAL)` checks as they're
> specific to one expected errno value, but this is no longer the case
> with ENOTDIR now in some fanotify_init/fanotify_mark cases.

Good point.

>
> Feel free to add RVB tags.

Thanks,
Amir.

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

Re: [LTP] [PATCH 1/2] syscalls/fanotify14: Encode the expected errno in test case

[Petr Vorel]

> On Tue, Jul 19, 2022 at 11:58:52AM +0200, Amir Goldstein wrote:
> > So we can add test cases for errors other than EINVAL.

> Just one optional nit. We can probably remove the comments which are
> directly above the existing `if (errno == EINVAL)` checks as they're
> specific to one expected errno value, but this is no longer the case
> with ENOTDIR now in some fanotify_init/fanotify_mark cases.

@Amir, I can remove it in this commit before merge (see diff below),
but don't you want to keep this info somewhere?
Or feel free to post new version.

BTW I tested both commits on various kernels and it works as expected
(failing only on 5.17.3 (openSUSE) 5.18.5, (Debian), old kernels TCONF
as expected.

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

> Feel free to add RVB tags.

@Matthew FYI adding it with correct form in the email reply help us maintainers
to get it added automatically via LTP patchwork instance.

Kind regards,
Petr

diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
index c99e19706..ce7e4c54c 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify14.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
@@ -84,11 +84,6 @@ static void do_test(unsigned int number)
 
 	fanotify_fd = fanotify_init(tc->init_flags, O_RDONLY);
 	if (fanotify_fd < 0) {
-		/*
-		 * EINVAL is to be returned to the calling process when
-		 * an invalid notification class is specified in
-		 * conjunction with FAN_REPORT_FID.
-		 */
 		if (errno == tc->expected_errno) {
 			tst_res(TPASS,
 				"fanotify_fd=%d, fanotify_init(%x, O_RDONLY) "
@@ -121,12 +116,6 @@ static void do_test(unsigned int number)
 	ret = fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark_flags,
 				tc->mask, AT_FDCWD, FILE1);
 	if (ret < 0) {
-		/*
-		 * EINVAL is to be returned to the calling process when
-		 * attempting to use INODE_EVENTS without FAN_REPORT_FID
-		 * specified on the notification group, or using
-		 * INODE_EVENTS with mark type FAN_MARK_MOUNT.
-		 */
 		if (errno == tc->expected_errno) {
 			tst_res(TPASS,
 				"ret=%d, fanotify_mark(%d, FAN_MARK_ADD | %x, "

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

Re: [LTP] [PATCH 1/2] syscalls/fanotify14: Encode the expected errno in test case

[Amir Goldstein]

On Mon, Jul 25, 2022 at 2:49 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> > On Tue, Jul 19, 2022 at 11:58:52AM +0200, Amir Goldstein wrote:
> > > So we can add test cases for errors other than EINVAL.
>
> > Just one optional nit. We can probably remove the comments which are
> > directly above the existing `if (errno == EINVAL)` checks as they're
> > specific to one expected errno value, but this is no longer the case
> > with ENOTDIR now in some fanotify_init/fanotify_mark cases.
>
> @Amir, I can remove it in this commit before merge (see diff below),
> but don't you want to keep this info somewhere?
> Or feel free to post new version.

Good idea.
I will add the comments to the individual test cases and re-post.

Thanks,
Amir.

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

[LTP] [PATCH 2/2] syscalls/fanotify14: Add test cases for events not allowed on non-dir

[Amir Goldstein]

Dirent events are meaningless in mask of non-dir inode and so do
the flags FAN_ONDIR FAN_EVENT_ON_CHILD.

With fanotify_init flag FAN_REPORT_TARGET_FID, they are also not allowed.
Make sure that trying to add them to non-dir inode mark mask results in
ENOTDIR error and that trying to add them to directory or filesystem
mark does not fail.

Also add validity check for legacy FAN_MARK_ONLYDIR and add more
information to error when safe_fanotify_mark() fails.

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

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 0ad7ef21b..a118fbd9e 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -48,7 +48,9 @@ static inline int safe_fanotify_mark(const char *file, const int lineno,
 	rval = fanotify_mark(fd, flags, mask, dfd, pathname);
 
 	if (rval == -1) {
-		tst_brk_(file, lineno, TBROK | TERRNO, "fanotify_mark() failed");
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			 "fanotify_mark(%d, 0x%x, 0x%lx, ..., %s) failed",
+			 fd, flags, mask, pathname);
 	}
 
 	if (rval < -1) {
diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
index c99e19706..0595d133c 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify14.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
@@ -13,6 +13,13 @@
  * mask value has been specified in conjunction with FAN_REPORT_FID.
  */
 
+/*
+ * The ENOTDIR test cases are regression tests for commits:
+ *
+ *     ceaf69f8eadc fanotify: do not allow setting dirent events in mask of non-dir
+ *     8698e3bab4dd fanotify: refine the validation checks on non-dir inode mask
+ */
+
 #define _GNU_SOURCE
 #include "tst_test.h"
 #include <errno.h>
@@ -31,6 +38,7 @@
 		      FAN_DELETE_SELF | FAN_MOVE_SELF)
 
 static int fanotify_fd;
+static int fan_report_target_fid_unsupported;
 
 /*
  * Each test case has been designed in a manner whereby the values defined
@@ -75,6 +83,26 @@ static struct test_case_t {
 		/* FAN_RENAME without FAN_REPORT_NAME is not valid */
 		FAN_CLASS_NOTIF | FAN_REPORT_DFID_FID, 0, FAN_RENAME, EINVAL
 	},
+	{
+		/* With FAN_MARK_ONLYDIR on non-dir is not valid */
+		FAN_CLASS_NOTIF, FAN_MARK_ONLYDIR, FAN_OPEN, ENOTDIR
+	},
+	{
+		/* With FAN_REPORT_TARGET_FID, FAN_DELETE on non-dir is not valid */
+		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_DELETE, ENOTDIR
+	},
+	{
+		/* With FAN_REPORT_TARGET_FID, FAN_RENAME on non-dir is not valid */
+		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_RENAME, ENOTDIR
+	},
+	{
+		/* With FAN_REPORT_TARGET_FID, FAN_ONDIR on non-dir is not valid */
+		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_OPEN | FAN_ONDIR, ENOTDIR
+	},
+	{
+		/* With FAN_REPORT_TARGET_FID, FAN_EVENT_ON_CHILD on non-dir is not valid */
+		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_OPEN | FAN_EVENT_ON_CHILD, ENOTDIR
+	},
 };
 
 static void do_test(unsigned int number)
@@ -82,6 +110,12 @@ static void do_test(unsigned int number)
 	int ret;
 	struct test_case_t *tc = &test_cases[number];
 
+	if (fan_report_target_fid_unsupported && tc->init_flags & FAN_REPORT_TARGET_FID) {
+		FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_TARGET_FID,
+					    fan_report_target_fid_unsupported);
+		return;
+	}
+
 	fanotify_fd = fanotify_init(tc->init_flags, O_RDONLY);
 	if (fanotify_fd < 0) {
 		/*
@@ -118,8 +152,10 @@ static void do_test(unsigned int number)
 		goto out;
 	}
 
+	/* Set mark on non-dir only when expecting error ENOTDIR */
+	const char *path = tc->expected_errno == ENOTDIR ? FILE1 : MNTPOINT;
 	ret = fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark_flags,
-				tc->mask, AT_FDCWD, FILE1);
+				tc->mask, AT_FDCWD, path);
 	if (ret < 0) {
 		/*
 		 * EINVAL is to be returned to the calling process when
@@ -136,7 +172,30 @@ static void do_test(unsigned int number)
 				fanotify_fd,
 				tc->mark_flags,
 				tc->mask,
-				FILE1, tc->expected_errno);
+				path, tc->expected_errno);
+			/*
+			 * ENOTDIR are errors for events/flags not allowed on a non-dir inode.
+			 * Try to set an inode mark on a directory and it should succeed.
+			 * Try to set directory events in filesystem mark mask on non-dir
+			 * and it should succeed.
+			 */
+			if (tc->expected_errno == ENOTDIR) {
+				SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD | tc->mark_flags,
+						   tc->mask, AT_FDCWD, MNTPOINT);
+				tst_res(TPASS,
+					"Adding an inode mark on directory did not fail with "
+					"ENOTDIR error as on non-dir inode");
+			}
+			if (tc->expected_errno == ENOTDIR &&
+			    !(tc->mark_flags & FAN_MARK_ONLYDIR)) {
+				SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD | tc->mark_flags |
+						   FAN_MARK_FILESYSTEM, tc->mask,
+						   AT_FDCWD, FILE1);
+				tst_res(TPASS,
+					"Adding a filesystem mark on non-dir did not fail with "
+					"ENOTDIR error as with an inode mark");
+			}
+
 			goto out;
 		}
 		tst_brk(TBROK | TERRNO,
@@ -172,6 +231,9 @@ static void do_setup(void)
 	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
 	SAFE_CLOSE(fd);
 
+	fan_report_target_fid_unsupported =
+		fanotify_init_flags_supported_on_fs(FAN_REPORT_DFID_NAME_TARGET, MNTPOINT);
+
 	/* Create temporary test file to place marks on */
 	SAFE_FILE_PRINTF(FILE1, "0");
 }
@@ -190,7 +252,12 @@ static struct tst_test test = {
 	.cleanup = do_cleanup,
 	.mount_device = 1,
 	.mntpoint = MNTPOINT,
-	.all_filesystems = 1
+	.all_filesystems = 1,
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "ceaf69f8eadc"},
+		{"linux-git", "8698e3bab4dd"},
+		{}
+	}
 };
 
 #else
-- 
2.25.1


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

Re: [LTP] [PATCH 2/2] syscalls/fanotify14: Add test cases for events not allowed on non-dir

[Matthew Bobrowski via ltp]

On Tue, Jul 19, 2022 at 11:58:53AM +0200, Amir Goldstein wrote:
> Dirent events are meaningless in mask of non-dir inode and so do
> the flags FAN_ONDIR FAN_EVENT_ON_CHILD.
> 
> With fanotify_init flag FAN_REPORT_TARGET_FID, they are also not allowed.
> Make sure that trying to add them to non-dir inode mark mask results in
> ENOTDIR error and that trying to add them to directory or filesystem
> mark does not fail.
> 
> Also add validity check for legacy FAN_MARK_ONLYDIR and add more
> information to error when safe_fanotify_mark() fails.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks OK to me. Feel free to add RVB tags.

> ---
>  testcases/kernel/syscalls/fanotify/fanotify.h |  4 +-
>  .../kernel/syscalls/fanotify/fanotify14.c     | 73 ++++++++++++++++++-
>  2 files changed, 73 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index 0ad7ef21b..a118fbd9e 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -48,7 +48,9 @@ static inline int safe_fanotify_mark(const char *file, const int lineno,
>  	rval = fanotify_mark(fd, flags, mask, dfd, pathname);
>  
>  	if (rval == -1) {
> -		tst_brk_(file, lineno, TBROK | TERRNO, "fanotify_mark() failed");
> +		tst_brk_(file, lineno, TBROK | TERRNO,
> +			 "fanotify_mark(%d, 0x%x, 0x%lx, ..., %s) failed",
> +			 fd, flags, mask, pathname);
>  	}
>  
>  	if (rval < -1) {
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
> index c99e19706..0595d133c 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify14.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
> @@ -13,6 +13,13 @@
>   * mask value has been specified in conjunction with FAN_REPORT_FID.
>   */
>  
> +/*
> + * The ENOTDIR test cases are regression tests for commits:
> + *
> + *     ceaf69f8eadc fanotify: do not allow setting dirent events in mask of non-dir
> + *     8698e3bab4dd fanotify: refine the validation checks on non-dir inode mask
> + */
> +
>  #define _GNU_SOURCE
>  #include "tst_test.h"
>  #include <errno.h>
> @@ -31,6 +38,7 @@
>  		      FAN_DELETE_SELF | FAN_MOVE_SELF)
>  
>  static int fanotify_fd;
> +static int fan_report_target_fid_unsupported;
>  
>  /*
>   * Each test case has been designed in a manner whereby the values defined
> @@ -75,6 +83,26 @@ static struct test_case_t {
>  		/* FAN_RENAME without FAN_REPORT_NAME is not valid */
>  		FAN_CLASS_NOTIF | FAN_REPORT_DFID_FID, 0, FAN_RENAME, EINVAL
>  	},
> +	{
> +		/* With FAN_MARK_ONLYDIR on non-dir is not valid */
> +		FAN_CLASS_NOTIF, FAN_MARK_ONLYDIR, FAN_OPEN, ENOTDIR
> +	},
> +	{
> +		/* With FAN_REPORT_TARGET_FID, FAN_DELETE on non-dir is not valid */
> +		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_DELETE, ENOTDIR
> +	},
> +	{
> +		/* With FAN_REPORT_TARGET_FID, FAN_RENAME on non-dir is not valid */
> +		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_RENAME, ENOTDIR
> +	},
> +	{
> +		/* With FAN_REPORT_TARGET_FID, FAN_ONDIR on non-dir is not valid */
> +		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_OPEN | FAN_ONDIR, ENOTDIR
> +	},
> +	{
> +		/* With FAN_REPORT_TARGET_FID, FAN_EVENT_ON_CHILD on non-dir is not valid */
> +		FAN_CLASS_NOTIF | FAN_REPORT_DFID_NAME_TARGET, 0, FAN_OPEN | FAN_EVENT_ON_CHILD, ENOTDIR
> +	},
>  };
>  
>  static void do_test(unsigned int number)
> @@ -82,6 +110,12 @@ static void do_test(unsigned int number)
>  	int ret;
>  	struct test_case_t *tc = &test_cases[number];
>  
> +	if (fan_report_target_fid_unsupported && tc->init_flags & FAN_REPORT_TARGET_FID) {
> +		FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_TARGET_FID,
> +					    fan_report_target_fid_unsupported);
> +		return;
> +	}
> +
>  	fanotify_fd = fanotify_init(tc->init_flags, O_RDONLY);
>  	if (fanotify_fd < 0) {
>  		/*
> @@ -118,8 +152,10 @@ static void do_test(unsigned int number)
>  		goto out;
>  	}
>  
> +	/* Set mark on non-dir only when expecting error ENOTDIR */
> +	const char *path = tc->expected_errno == ENOTDIR ? FILE1 : MNTPOINT;
>  	ret = fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark_flags,
> -				tc->mask, AT_FDCWD, FILE1);
> +				tc->mask, AT_FDCWD, path);
>  	if (ret < 0) {
>  		/*
>  		 * EINVAL is to be returned to the calling process when
> @@ -136,7 +172,30 @@ static void do_test(unsigned int number)
>  				fanotify_fd,
>  				tc->mark_flags,
>  				tc->mask,
> -				FILE1, tc->expected_errno);
> +				path, tc->expected_errno);
> +			/*
> +			 * ENOTDIR are errors for events/flags not allowed on a non-dir inode.
> +			 * Try to set an inode mark on a directory and it should succeed.
> +			 * Try to set directory events in filesystem mark mask on non-dir
> +			 * and it should succeed.
> +			 */
> +			if (tc->expected_errno == ENOTDIR) {
> +				SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD | tc->mark_flags,
> +						   tc->mask, AT_FDCWD, MNTPOINT);
> +				tst_res(TPASS,
> +					"Adding an inode mark on directory did not fail with "
> +					"ENOTDIR error as on non-dir inode");
> +			}
> +			if (tc->expected_errno == ENOTDIR &&
> +			    !(tc->mark_flags & FAN_MARK_ONLYDIR)) {
> +				SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD | tc->mark_flags |
> +						   FAN_MARK_FILESYSTEM, tc->mask,
> +						   AT_FDCWD, FILE1);
> +				tst_res(TPASS,
> +					"Adding a filesystem mark on non-dir did not fail with "
> +					"ENOTDIR error as with an inode mark");
> +			}
> +
>  			goto out;
>  		}
>  		tst_brk(TBROK | TERRNO,
> @@ -172,6 +231,9 @@ static void do_setup(void)
>  	fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
>  	SAFE_CLOSE(fd);
>  
> +	fan_report_target_fid_unsupported =
> +		fanotify_init_flags_supported_on_fs(FAN_REPORT_DFID_NAME_TARGET, MNTPOINT);
> +
>  	/* Create temporary test file to place marks on */
>  	SAFE_FILE_PRINTF(FILE1, "0");
>  }
> @@ -190,7 +252,12 @@ static struct tst_test test = {
>  	.cleanup = do_cleanup,
>  	.mount_device = 1,
>  	.mntpoint = MNTPOINT,
> -	.all_filesystems = 1
> +	.all_filesystems = 1,
> +	.tags = (const struct tst_tag[]) {
> +		{"linux-git", "ceaf69f8eadc"},
> +		{"linux-git", "8698e3bab4dd"},
> +		{}
> +	}
>  };
>  
>  #else
> -- 
> 2.25.1

/M

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

Re: [LTP] [PATCH 2/2] syscalls/fanotify14: Add test cases for events not allowed on non-dir

[Petr Vorel]

Hi all,

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

Kind regards,
Petr

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