Skip to contentSkip to conversation

syscalls/fanotify26: Add directory cases for the nested event

PATCH In review Edvard Conrad via ltp 4 participants 6 messages Oct 31 – Nov 3

lore
Share & tools
lore

Edvard Conrad via ltp

author
#0 lore raw

PATCH 1/1 syscalls/fanotify26: Add directory cases for the nested event

A directory on overlayfs reaches its real directory through ovl_dir_open_realfile() rather than backing_file_open(), so the fix for regular files, commit c3d355f427ce ("fs: let backing files inherit FMODE_NONOTIFY from the user file"), does not cover it: a listener that marks the backing filesystem with FAN_OPEN_PERM | FAN_ONDIR is still sent a nested permission event from its own read(2) when the reported path is an overlay directory.

Add the directory counterparts of the two existing cases. The listener's expected event mask stays FAN_OPEN_PERM: FAN_ONDIR is not reported to a group without FAN_REPORT_FID.

The fourth case fails on a kernel without the pending fix "ovl:
inherit FMODE_NONOTIFY when opening directory realfiles".

Signed-off-by Edvard Conrad <[email protected]>

---
Depends on the pending overlayfs fix, posted by Amir on the kernel thread:
https://lore.kernel.org/all/CAOQ4uxjm=cGPnug18VuhKqiMvkWwLPNLO_TE3KxO8iLZG+veYQ@mail.gmail.com/
Without it the fourth case fails on current mainline, so this should wait in the queue until that fix is in Linus' tree; a linux-git tag for it can then be added. The first three cases pass on v7.4-rc1 material today.

Tested in a VM, each kernel run once and with -i 2:

  kernel                              file+backing  dir+backing
  v6.13                               PASS          FAIL
  6b4b543fa18 (no fix)                FAIL          FAIL
  + c3d355f427ce (files, in Linus')   PASS          FAIL
  + the pending ovl fix               PASS          PASS

The "overlay mark only" cases pass everywhere. The directory case fails on v6.13 too: overlayfs never passed the nonotify state on to the real directory, so it predates the 6.14 change that broke files.

 .../kernel/syscalls/fanotify/fanotify26.c     | 43 +++++++++++++------
 1 file changed, 30 insertions(+), 13 deletions(-)
testcases/kernel/syscalls/fanotify/fanotify26.c +30 -13
index 63d7eea0f..3f6b525ef 100644
@@ -17,9 +17,11 @@
  * listener that reads with a single thread never returns, and every task that
  * touches a watched filesystem queues up behind it.
  *
- * The first test case marks the overlay only and must pass on any kernel. The
- * second one marks the backing filesystem as well, which is the shape an
- * anti-malware scanner uses on an overlayfs root.
+ * Regular files and directories reach their real file through different
+ * paths in overlayfs, so both are tested. For each, the first test case marks
+ * the overlay only and must pass on any kernel, the second one marks the
+ * backing filesystem as well, which is the shape an anti-malware scanner uses
+ * on an overlayfs root.
  *
  * The test needs root to create permission events and to mount overlayfs.
  */

@@ -39,8 +41,11 @@
 
 #define BACKING_MNT	OVL_BASE_MNTPOINT
 #define FNAME		"test_file"
+#define DNAME		"test_dir"
 #define OVL_FILE	OVL_MNT "/" FNAME
+#define OVL_DIR		OVL_MNT "/" DNAME
 #define LOWER_FILE	OVL_LOWER "/" FNAME
+#define LOWER_DIR	OVL_LOWER "/" DNAME
 
 /*
  * The listener either answers the event at once or is blocked for good, so

@@ -55,9 +60,12 @@ static int fd_notify = -1;
 static struct tcase {
 	const char *tname;
 	int mark_backing;
+	int ondir;
 } tcases[] = {
-	{"Mark on the overlay", 0},
-	{"Mark on the overlay and on the backing filesystem", 1},
+	{"Overlay mark, file", 0, 0},
+	{"Overlay and backing mark, file", 1, 0},
+	{"Overlay mark, directory", 0, 1},
+	{"Overlay and backing mark, directory", 1, 1},
 };
 
 static void answer_one_event(pid_t opener)

@@ -71,7 +79,9 @@ static void answer_one_event(pid_t opener)
 	/*
 	 * A wrong mask or a stranger's event is a result, not harness
 	 * breakage, so report and still answer: the opener stays parked in
-	 * the kernel until something does.
+	 * the kernel until something does. FAN_ONDIR is not reported to a
+	 * group without FAN_REPORT_FID, so a directory open arrives as a
+	 * plain FAN_OPEN_PERM as well.
 	 */
 	if (event.mask != FAN_OPEN_PERM) {
 		tst_res(TFAIL, "Got event mask %llx, expected %llx",

@@ -95,6 +105,7 @@ static void answer_one_event(pid_t opener)
 static void run(unsigned int n)
 {
 	struct tcase *tc = &tcases[n];
+	uint64_t mask = FAN_OPEN_PERM | (tc->ondir ? FAN_ONDIR : 0);
 	pid_t opener, reader;
 
 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);

@@ -102,12 +113,12 @@ static void run(unsigned int n)
 	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
 
 	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | FAN_MARK_FILESYSTEM,
-			   FAN_OPEN_PERM, AT_FDCWD, OVL_MNT);
+			   mask, AT_FDCWD, OVL_MNT);
 
 	if (tc->mark_backing) {
 		SAFE_FANOTIFY_MARK(fd_notify,
 				   FAN_MARK_ADD | FAN_MARK_FILESYSTEM,
-				   FAN_OPEN_PERM, AT_FDCWD, BACKING_MNT);
+				   mask, AT_FDCWD, BACKING_MNT);
 	}
 
 	opener = SAFE_FORK();

@@ -121,7 +132,10 @@ static void run(unsigned int n)
 		 * answered it goes away, so a failure here is the fallout of
 		 * the test tearing a blocked listener down, not a result.
 		 */
-		fd = open(OVL_FILE, O_RDONLY);
+		if (tc->ondir)
+			fd = open(OVL_DIR, O_RDONLY | O_DIRECTORY);
+		else
+			fd = open(OVL_FILE, O_RDONLY);
 		if (fd != -1)
 			SAFE_CLOSE(fd);
 		exit(0);

@@ -161,18 +175,21 @@ static void run(unsigned int n)
 
 static void setup(void)
 {
-	/* The file has to exist in the lower layer to be opened through it. */
+	/* Both have to exist in the lower layer to be opened through it. */
 	SAFE_UMOUNT(OVL_MNT);
 	SAFE_TOUCH(LOWER_FILE, 0644, NULL);
+	SAFE_MKDIR(LOWER_DIR, 0755);
 	SAFE_MOUNT_OVERLAY();
 
 	/*
-	 * Probe the combination run() uses, not a weaker proxy for it. Every
-	 * case marks the overlay; the second also marks the base.
+	 * Probe the widest combination run() uses, not a weaker proxy for
+	 * it. Every case marks the overlay; the second of each pair also
+	 * marks the base.
 	 */
 	REQUIRE_FANOTIFY_EVENTS_SUPPORTED_ON_FS(FAN_CLASS_CONTENT,
 						FAN_MARK_FILESYSTEM,
-						FAN_OPEN_PERM, OVL_MNT);
+						FAN_OPEN_PERM | FAN_ONDIR,
+						OVL_MNT);
 	REQUIRE_MARK_TYPE_SUPPORTED_ON_FS(FAN_MARK_FILESYSTEM, BACKING_MNT);
 }
Signature · 5 lines
2.56.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp
#1 lore raw

Hi Edvard,

Quoted 1 line · Edvard Conrad

syscalls/fanotify26: Add directory cases for the nested event

Verdict - Reviewed

---
Note:

The agent can sometimes produce false positives although often its findings are genuine. If you find issues with the review, please comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

Signature · 1 line
Mailing list info: https://lists.linux.it/listinfo/ltp
Reviewed-by on v1 1/1
#2 lore raw
In reply to Edvard Conrad via ltp
Quoted 9 lines · Edvard Conrad

A directory on overlayfs reaches its real directory through ovl_dir_open_realfile() rather than backing_file_open(), so the fix for regular files, commit c3d355f427ce ("fs: let backing files inherit FMODE_NONOTIFY from the user file"), does not cover it: a listener that marks the backing filesystem with FAN_OPEN_PERM | FAN_ONDIR is still sent a nested permission event from its own read(2) when the reported path is an overlay directory.

Quoted 8 lines

Add the directory counterparts of the two existing cases. The listener's expected event mask stays FAN_OPEN_PERM: FAN_ONDIR is not reported to a group without FAN_REPORT_FID.

The fourth case fails on a kernel without the pending fix "ovl:
inherit FMODE_NONOTIFY when opening directory realfiles".

Signed-off-by Edvard Conrad <[email protected]>
Reviewed-by Amir Goldstein <[email protected]>
Quoted 17 lines

---
Depends on the pending overlayfs fix, posted by Amir on the kernel thread:
https://lore.kernel.org/all/CAOQ4uxjm=cGPnug18VuhKqiMvkWwLPNLO_TE3KxO8iLZG+veYQ@mail.gmail.com/
Without it the fourth case fails on current mainline, so this should wait in the queue until that fix is in Linus' tree; a linux-git tag for it can then be added. The first three cases pass on v7.4-rc1 material today.

Tested in a VM, each kernel run once and with -i 2:

  kernel                              file+backing  dir+backing
  v6.13                               PASS          FAIL
  6b4b543fa18 (no fix)                FAIL          FAIL
  + c3d355f427ce (files, in Linus')   PASS          FAIL
  + the pending ovl fix               PASS          PASS
Quoted 3 lines

The "overlay mark only" cases pass everywhere. The directory case fails on v6.13 too: overlayfs never passed the nonotify state on to the real directory, so it predates the 6.14 change that broke files.

As far as I can tell its a regression from
130fdbc3d1f99 ("ovl: pass correct flags for opening real directory")

Thanks for adding this test.
I added your Tested-by to the fix patch.

Thanks,
Amir.

Quoted 125 lines
 .../kernel/syscalls/fanotify/fanotify26.c     | 43 +++++++++++++------
 1 file changed, 30 insertions(+), 13 deletions(-)
testcases/kernel/syscalls/fanotify/fanotify26.c +30 -14
index 63d7eea0f..3f6b525ef 100644
@@ -17,9 +17,11 @@
  * listener that reads with a single thread never returns, and every task that
  * touches a watched filesystem queues up behind it.
  *
- * The first test case marks the overlay only and must pass on any kernel. The
- * second one marks the backing filesystem as well, which is the shape an
- * anti-malware scanner uses on an overlayfs root.
+ * Regular files and directories reach their real file through different
+ * paths in overlayfs, so both are tested. For each, the first test case marks
+ * the overlay only and must pass on any kernel, the second one marks the
+ * backing filesystem as well, which is the shape an anti-malware scanner uses
+ * on an overlayfs root.
  *
  * The test needs root to create permission events and to mount overlayfs.
  */

@@ -39,8 +41,11 @@
 
 #define BACKING_MNT    OVL_BASE_MNTPOINT
 #define FNAME          "test_file"
+#define DNAME          "test_dir"
 #define OVL_FILE       OVL_MNT "/" FNAME
+#define OVL_DIR                OVL_MNT "/" DNAME
 #define LOWER_FILE     OVL_LOWER "/" FNAME
+#define LOWER_DIR      OVL_LOWER "/" DNAME
 
 /*
  * The listener either answers the event at once or is blocked for good, so

@@ -55,9 +60,12 @@ static int fd_notify = -1;
 static struct tcase {
        const char *tname;
        int mark_backing;
+       int ondir;
 } tcases[] = {
-       {"Mark on the overlay", 0},
-       {"Mark on the overlay and on the backing filesystem", 1},
+       {"Overlay mark, file", 0, 0},
+       {"Overlay and backing mark, file", 1, 0},
+       {"Overlay mark, directory", 0, 1},
+       {"Overlay and backing mark, directory", 1, 1},
 };
 
 static void answer_one_event(pid_t opener)

@@ -71,7 +79,9 @@ static void answer_one_event(pid_t opener)
        /*
         * A wrong mask or a stranger's event is a result, not harness
         * breakage, so report and still answer: the opener stays parked in
-        * the kernel until something does.
+        * the kernel until something does. FAN_ONDIR is not reported to a
+        * group without FAN_REPORT_FID, so a directory open arrives as a
+        * plain FAN_OPEN_PERM as well.
         */
        if (event.mask != FAN_OPEN_PERM) {
                tst_res(TFAIL, "Got event mask %llx, expected %llx",

@@ -95,6 +105,7 @@ static void answer_one_event(pid_t opener)
 static void run(unsigned int n)
 {
        struct tcase *tc = &tcases[n];
+       uint64_t mask = FAN_OPEN_PERM | (tc->ondir ? FAN_ONDIR : 0);
        pid_t opener, reader;
 
        tst_res(TINFO, "Test #%d: %s", n, tc->tname);

@@ -102,12 +113,12 @@ static void run(unsigned int n)
        fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
 
        SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | FAN_MARK_FILESYSTEM,
-                          FAN_OPEN_PERM, AT_FDCWD, OVL_MNT);
+                          mask, AT_FDCWD, OVL_MNT);
 
        if (tc->mark_backing) {
                SAFE_FANOTIFY_MARK(fd_notify,
                                   FAN_MARK_ADD | FAN_MARK_FILESYSTEM,
-                                  FAN_OPEN_PERM, AT_FDCWD, BACKING_MNT);
+                                  mask, AT_FDCWD, BACKING_MNT);
        }
 
        opener = SAFE_FORK();

@@ -121,7 +132,10 @@ static void run(unsigned int n)
                 * answered it goes away, so a failure here is the fallout of
                 * the test tearing a blocked listener down, not a result.
                 */
-               fd = open(OVL_FILE, O_RDONLY);
+               if (tc->ondir)
+                       fd = open(OVL_DIR, O_RDONLY | O_DIRECTORY);
+               else
+                       fd = open(OVL_FILE, O_RDONLY);
                if (fd != -1)
                        SAFE_CLOSE(fd);
                exit(0);

@@ -161,18 +175,21 @@ static void run(unsigned int n)
 
 static void setup(void)
 {
-       /* The file has to exist in the lower layer to be opened through it. */
+       /* Both have to exist in the lower layer to be opened through it. */
        SAFE_UMOUNT(OVL_MNT);
        SAFE_TOUCH(LOWER_FILE, 0644, NULL);
+       SAFE_MKDIR(LOWER_DIR, 0755);
        SAFE_MOUNT_OVERLAY();
 
        /*
-        * Probe the combination run() uses, not a weaker proxy for it. Every
-        * case marks the overlay; the second also marks the base.
+        * Probe the widest combination run() uses, not a weaker proxy for
+        * it. Every case marks the overlay; the second of each pair also
+        * marks the base.
         */
        REQUIRE_FANOTIFY_EVENTS_SUPPORTED_ON_FS(FAN_CLASS_CONTENT,
                                                FAN_MARK_FILESYSTEM,
-                                               FAN_OPEN_PERM, OVL_MNT);
+                                               FAN_OPEN_PERM | FAN_ONDIR,
+                                               OVL_MNT);
        REQUIRE_MARK_TYPE_SUPPORTED_ON_FS(FAN_MARK_FILESYSTEM, BACKING_MNT);
 }
 
--

2.56.0

Signature · 1 line
Mailing list info: https://lists.linux.it/listinfo/ltp
Reviewed-by
#3 lore raw

Hi Amir, Edvard,

Quoted 42 lines · Edvard Conrad
Quoted 9 lines

A directory on overlayfs reaches its real directory through ovl_dir_open_realfile() rather than backing_file_open(), so the fix for regular files, commit c3d355f427ce ("fs: let backing files inherit FMODE_NONOTIFY from the user file"), does not cover it: a listener that marks the backing filesystem with FAN_OPEN_PERM | FAN_ONDIR is still sent a nested permission event from its own read(2) when the reported path is an overlay directory.

Quoted 8 lines

Add the directory counterparts of the two existing cases. The listener's expected event mask stays FAN_OPEN_PERM: FAN_ONDIR is not reported to a group without FAN_REPORT_FID.

The fourth case fails on a kernel without the pending fix "ovl:
inherit FMODE_NONOTIFY when opening directory realfiles".

Signed-off-by Edvard Conrad <[email protected]>
Reviewed-by Amir Goldstein <[email protected]>
Quoted 17 lines

---
Depends on the pending overlayfs fix, posted by Amir on the kernel thread:
https://lore.kernel.org/all/CAOQ4uxjm=cGPnug18VuhKqiMvkWwLPNLO_TE3KxO8iLZG+veYQ@mail.gmail.com/
Without it the fourth case fails on current mainline, so this should wait in the queue until that fix is in Linus' tree; a linux-git tag for it can then be added. The first three cases pass on v7.4-rc1 material today.

Tested in a VM, each kernel run once and with -i 2:

  kernel                              file+backing  dir+backing
  v6.13                               PASS          FAIL
  6b4b543fa18 (no fix)                FAIL          FAIL
  + c3d355f427ce (files, in Linus')   PASS          FAIL
  + the pending ovl fix               PASS          PASS
Quoted 3 lines

The "overlay mark only" cases pass everywhere. The directory case fails on v6.13 too: overlayfs never passed the nonotify state on to the real directory, so it predates the 6.14 change that broke files.

Quoted 5 lines

As far as I can tell its a regression from
130fdbc3d1f99 ("ovl: pass correct flags for opening real directory")

Thanks for adding this test.
I added your Tested-by to the fix patch.

I'm going to merge today. So there will be hash for fixing kernel commit, thanks for info.

Reviewed-by Petr Vorel <[email protected]>
Quoted 2 lines

Thanks,
Amir.

Signature · 1 line
Mailing list info: https://lists.linux.it/listinfo/ltp
#4 lore raw
Quoted 45 lines · Petr Vorel

Hi Amir, Edvard,

Quoted 41 lines · Edvard Conrad
Quoted 9 lines

A directory on overlayfs reaches its real directory through ovl_dir_open_realfile() rather than backing_file_open(), so the fix for regular files, commit c3d355f427ce ("fs: let backing files inherit FMODE_NONOTIFY from the user file"), does not cover it: a listener that marks the backing filesystem with FAN_OPEN_PERM | FAN_ONDIR is still sent a nested permission event from its own read(2) when the reported path is an overlay directory.

Quoted 8 lines

Add the directory counterparts of the two existing cases. The listener's expected event mask stays FAN_OPEN_PERM: FAN_ONDIR is not reported to a group without FAN_REPORT_FID.

The fourth case fails on a kernel without the pending fix "ovl:
inherit FMODE_NONOTIFY when opening directory realfiles".

Signed-off-by Edvard Conrad <[email protected]>
Reviewed-by Amir Goldstein <[email protected]>
Quoted 20 lines

---
Depends on the pending overlayfs fix, posted by Amir on the kernel thread:
https://lore.kernel.org/all/CAOQ4uxjm=cGPnug18VuhKqiMvkWwLPNLO_TE3KxO8iLZG+veYQ@mail.gmail.com/
Without it the fourth case fails on current mainline, so this should wait in the queue until that fix is in Linus' tree; a linux-git tag for it can then be added. The first three cases pass on v7.4-rc1 material today.

Tested in a VM, each kernel run once and with -i 2:

  kernel                              file+backing  dir+backing
  v6.13                               PASS          FAIL
  6b4b543fa18 (no fix)                FAIL          FAIL
  + c3d355f427ce (files, in Linus')   PASS          FAIL
  + the pending ovl fix               PASS          PASS

The "overlay mark only" cases pass everywhere. The directory case fails on v6.13 too: overlayfs never passed the nonotify state on to the real directory, so it predates the 6.14 change that broke files.

Quoted 9 lines
Quoted 5 lines

As far as I can tell its a regression from
130fdbc3d1f99 ("ovl: pass correct flags for opening real directory")

Thanks for adding this test.
I added your Tested-by to the fix patch.

I'm going to merge today. So there will be hash for fixing kernel commit, thanks for info.

FYI, Edvard found another bug and has written an extra test case to cover it the expected kernel fix is going to cover the directory test cases in this patch as well as the extra test case:

https://lore.kernel.org/linux-unionfs/[email protected]/

Up to you if you want to merge now or wait for the extra test case.

Thanks,
Amir.

Signature · 1 line
Mailing list info: https://lists.linux.it/listinfo/ltp
#5 lore raw
Quoted 56 lines · Petr Vorel
Quoted 45 lines

Hi Amir, Edvard,

Quoted 41 lines · Edvard Conrad
Quoted 9 lines

A directory on overlayfs reaches its real directory through ovl_dir_open_realfile() rather than backing_file_open(), so the fix for regular files, commit c3d355f427ce ("fs: let backing files inherit FMODE_NONOTIFY from the user file"), does not cover it: a listener that marks the backing filesystem with FAN_OPEN_PERM | FAN_ONDIR is still sent a nested permission event from its own read(2) when the reported path is an overlay directory.

Quoted 8 lines

Add the directory counterparts of the two existing cases. The listener's expected event mask stays FAN_OPEN_PERM: FAN_ONDIR is not reported to a group without FAN_REPORT_FID.

The fourth case fails on a kernel without the pending fix "ovl:
inherit FMODE_NONOTIFY when opening directory realfiles".

Signed-off-by Edvard Conrad <[email protected]>
Reviewed-by Amir Goldstein <[email protected]>
Quoted 20 lines

---
Depends on the pending overlayfs fix, posted by Amir on the kernel thread:
https://lore.kernel.org/all/CAOQ4uxjm=cGPnug18VuhKqiMvkWwLPNLO_TE3KxO8iLZG+veYQ@mail.gmail.com/
Without it the fourth case fails on current mainline, so this should wait in the queue until that fix is in Linus' tree; a linux-git tag for it can then be added. The first three cases pass on v7.4-rc1 material today.

Tested in a VM, each kernel run once and with -i 2:

  kernel                              file+backing  dir+backing
  v6.13                               PASS          FAIL
  6b4b543fa18 (no fix)                FAIL          FAIL
  + c3d355f427ce (files, in Linus')   PASS          FAIL
  + the pending ovl fix               PASS          PASS

The "overlay mark only" cases pass everywhere. The directory case fails on v6.13 too: overlayfs never passed the nonotify state on to the real directory, so it predates the 6.14 change that broke files.

Quoted 8 lines
Quoted 5 lines

As far as I can tell its a regression from
130fdbc3d1f99 ("ovl: pass correct flags for opening real directory")

Thanks for adding this test.
I added your Tested-by to the fix patch.

I'm going to merge today. So there will be hash for fixing kernel commit, thanks for info.

Quoted 5 lines

FYI, Edvard found another bug and has written an extra test case to cover it the expected kernel fix is going to cover the directory test cases in this patch as well as the extra test case:

https://lore.kernel.org/linux-unionfs/[email protected]/

Thanks a lot for info!

Quoted 1 line

Up to you if you want to merge now or wait for the extra test case.

I merged that one, I suppose there will be more patches (more testcases, git hash of the fix).

Kind regards,
Petr

Quoted 2 lines

Thanks,
Amir.

Signature · 1 line
Mailing list info: https://lists.linux.it/listinfo/ltp