Linux Test Project
 help / color / mirror / Atom feed
* [LTP] [PATCH] syscalls/fanotify26: Add cases that list the directory
@ 2026-11-03 12:14 Edvard Conrad via ltp
  2026-11-03 14:44 ` Amir Goldstein
  2026-11-03 18:13 ` [LTP] " linuxtestproject.agent
  0 siblings, 2 replies; 4+ messages in thread
From: Edvard Conrad via ltp @ 2026-11-03 12:14 UTC (permalink / raw)
  To: ltp@lists.linux.it; +Cc: Amir Goldstein, Jan Kara, Edvard Conrad

Listing a directory through the event file descriptor makes overlayfs
open its layers again, after the event fd exists: every layer of a
merged directory in ovl_dir_read_merged(), and the upper directory of
an impure one in ovl_dir_read_impure(). Unless those opens inherit
FMODE_NONOTIFY from the event fd, a listener that marks the backing
filesystem and lists the directory before it answers is sent a nested
permission event and blocks in getdents64().

Add a case for each. The impure directory is made by renaming a lower
file into a directory created on the overlay. The case reports TCONF
when the overlay cannot record the file's origin and so does not mark
the directory impure. The impure listing is cached with the inode, so
the case moves the entry out and back first; otherwise a later
iteration would read the cache instead of the layer.

Both cases fail on a kernel without the pending overlayfs fix for
directory opens.

Signed-off-by: Edvard Conrad <edvard@tuta.io>
---
Applies on top of "syscalls/fanotify26: Add directory cases for the
nested event":
  https://lore.kernel.org/ltp/P5Iny41--F-9@tuta.io/

Asked for by Amir on the kernel thread, where both paths came up:
  https://lore.kernel.org/all/CAOQ4uxh67+vs0mr-Lxmwz9-N+F8m29WUNz+8qFLLDpOVYB7=vg@mail.gmail.com/

Tested in a VM on 6b4b543fa18, each kernel run once and with -i 2,
with the backing filesystem marked:

                                file  dir   merged dir  impure dir
                                            listed      listed
  no fix                        FAIL  FAIL  FAIL        FAIL
  + c3d355f427ce                PASS  FAIL  FAIL        FAIL
  + ovl directory fix, v1       PASS  PASS  FAIL        FAIL
  + ovl directory fix, v2       PASS  PASS  PASS        FAIL
  + v2 and the follow-up on     PASS  PASS  PASS        PASS
    the kernel thread

The overlay-only cases pass on all of them. Without the fix the impure
case fails in both iterations of -i 2, which is what moving the entry
out and back is for.

 .../kernel/syscalls/fanotify/fanotify26.c     | 92 ++++++++++++++++---
 1 file changed, 79 insertions(+), 13 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify26.c b/testcases/kernel/syscalls/fanotify/fanotify26.c
index 3f6b525ef..f6a56821f 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify26.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify26.c
@@ -23,6 +23,11 @@
  * backing filesystem as well, which is the shape an anti-malware scanner uses
  * on an overlayfs root.
  *
+ * Listing a directory through the event file descriptor can make overlayfs
+ * open its layers again: every layer of a merged directory, and the upper
+ * directory of an impure one, which holds entries moved in from below. The
+ * last two test cases have the listener do that before it answers.
+ *
  * The test needs root to create permission events and to mount overlayfs.
  */
 
@@ -34,7 +39,9 @@
 #include <signal.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <sys/xattr.h>
 #include "tst_test.h"
+#include "lapi/syscalls.h"
 
 #ifdef HAVE_SYS_FANOTIFY_H
 #include "fanotify.h"
@@ -42,10 +49,21 @@
 #define BACKING_MNT	OVL_BASE_MNTPOINT
 #define FNAME		"test_file"
 #define DNAME		"test_dir"
+#define MNAME		"merged_dir"
+#define INAME		"impure_dir"
+#define MOVED		"moved_file"
 #define OVL_FILE	OVL_MNT "/" FNAME
 #define OVL_DIR		OVL_MNT "/" DNAME
+#define OVL_MDIR	OVL_MNT "/" MNAME
+#define OVL_IDIR	OVL_MNT "/" INAME
+#define OVL_MOVED	OVL_MNT "/" MOVED
+#define OVL_IDIR_MOVED	OVL_IDIR "/" MOVED
 #define LOWER_FILE	OVL_LOWER "/" FNAME
 #define LOWER_DIR	OVL_LOWER "/" DNAME
+#define LOWER_MDIR	OVL_LOWER "/" MNAME
+#define LOWER_MOVED	OVL_LOWER "/" MOVED
+#define UPPER_MDIR	OVL_UPPER "/" MNAME
+#define UPPER_IDIR	OVL_UPPER "/" INAME
 
 /*
  * The listener either answers the event at once or is blocked for good, so
@@ -56,19 +74,41 @@
 #define WAIT_MSEC	5000
 
 static int fd_notify = -1;
+static int idir_is_impure;
 
 static struct tcase {
 	const char *tname;
+	const char *path;
 	int mark_backing;
 	int ondir;
+	int list;
+	int impure;
 } tcases[] = {
-	{"Overlay mark, file", 0, 0},
-	{"Overlay and backing mark, file", 1, 0},
-	{"Overlay mark, directory", 0, 1},
-	{"Overlay and backing mark, directory", 1, 1},
+	{"Overlay mark, file", OVL_FILE, 0, 0, 0, 0},
+	{"Overlay and backing mark, file", OVL_FILE, 1, 0, 0, 0},
+	{"Overlay mark, directory", OVL_DIR, 0, 1, 0, 0},
+	{"Overlay and backing mark, directory", OVL_DIR, 1, 1, 0, 0},
+	{"Overlay and backing mark, merged directory, listed",
+	 OVL_MDIR, 1, 1, 1, 0},
+	{"Overlay and backing mark, impure directory, listed",
+	 OVL_IDIR, 1, 1, 1, 1},
 };
 
-static void answer_one_event(pid_t opener)
+static void list_dir(int fd)
+{
+	char buf[1024];
+	long ret;
+
+	/* Blocks for good if reading a layer generates a nested event. */
+	do {
+		ret = tst_syscall(__NR_getdents64, fd, buf, sizeof(buf));
+	} while (ret > 0);
+
+	if (ret == -1)
+		tst_brk(TBROK | TERRNO, "getdents64() on the event fd failed");
+}
+
+static void answer_one_event(pid_t opener, int list)
 {
 	struct fanotify_event_metadata event;
 	struct fanotify_response resp;
@@ -76,6 +116,9 @@ static void answer_one_event(pid_t opener)
 	/* Blocks for good if the event fd generates a nested event. */
 	SAFE_READ(SAFE_READ_ALL, fd_notify, &event, sizeof(event));
 
+	if (list && event.fd != FAN_NOFD)
+		list_dir(event.fd);
+
 	/*
 	 * A wrong mask or a stranger's event is a result, not harness
 	 * breakage, so report and still answer: the opener stays parked in
@@ -110,6 +153,20 @@ static void run(unsigned int n)
 
 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
 
+	if (tc->impure) {
+		if (!idir_is_impure) {
+			tst_res(TCONF, "Overlay did not mark the directory impure");
+			return;
+		}
+		/*
+		 * The impure listing is cached with the inode and reused, so
+		 * a later iteration would not open the layer at all. Moving
+		 * an entry with an origin out and back invalidates it.
+		 */
+		SAFE_RENAME(OVL_IDIR_MOVED, OVL_MOVED);
+		SAFE_RENAME(OVL_MOVED, OVL_IDIR_MOVED);
+	}
+
 	fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
 
 	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | FAN_MARK_FILESYSTEM,
@@ -132,10 +189,7 @@ 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.
 		 */
-		if (tc->ondir)
-			fd = open(OVL_DIR, O_RDONLY | O_DIRECTORY);
-		else
-			fd = open(OVL_FILE, O_RDONLY);
+		fd = open(tc->path, O_RDONLY | (tc->ondir ? O_DIRECTORY : 0));
 		if (fd != -1)
 			SAFE_CLOSE(fd);
 		exit(0);
@@ -148,7 +202,7 @@ static void run(unsigned int n)
 
 	reader = SAFE_FORK();
 	if (!reader) {
-		answer_one_event(opener);
+		answer_one_event(opener, tc->list);
 		exit(0);
 	}
 
@@ -160,8 +214,7 @@ static void run(unsigned int n)
 	 */
 	if (TST_PROCESS_STATE_WAIT(reader, 'Z',
 				   tst_multiply_timeout(WAIT_MSEC))) {
-		tst_res(TFAIL,
-			"Listener blocked in read() by a nested event");
+		tst_res(TFAIL, "Listener blocked by a nested event");
 		SAFE_KILL(reader, SIGKILL);
 		SAFE_WAITPID(reader, NULL, 0);
 	}
@@ -175,12 +228,25 @@ static void run(unsigned int n)
 
 static void setup(void)
 {
-	/* Both have to exist in the lower layer to be opened through it. */
+	/* These have to exist in the layers to be opened through them. */
 	SAFE_UMOUNT(OVL_MNT);
 	SAFE_TOUCH(LOWER_FILE, 0644, NULL);
 	SAFE_MKDIR(LOWER_DIR, 0755);
+	SAFE_MKDIR(LOWER_MDIR, 0755);
+	SAFE_MKDIR(UPPER_MDIR, 0755);
+	SAFE_TOUCH(LOWER_MOVED, 0644, NULL);
 	SAFE_MOUNT_OVERLAY();
 
+	/*
+	 * A directory made through the overlay is upper only. Moving a lower
+	 * file into it records the file's origin and marks the directory
+	 * impure, unless the layers cannot carry an origin.
+	 */
+	SAFE_MKDIR(OVL_IDIR, 0755);
+	SAFE_RENAME(OVL_MOVED, OVL_IDIR_MOVED);
+	idir_is_impure =
+		getxattr(UPPER_IDIR, "trusted.overlay.impure", NULL, 0) != -1;
+
 	/*
 	 * Probe the widest combination run() uses, not a weaker proxy for
 	 * it. Every case marks the overlay; the second of each pair also
-- 
2.56.0


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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH] syscalls/fanotify26: Add cases that list the directory
  2026-11-03 12:14 [LTP] [PATCH] syscalls/fanotify26: Add cases that list the directory Edvard Conrad via ltp
@ 2026-11-03 14:44 ` Amir Goldstein
  2026-11-03 19:44   ` Petr Vorel
  2026-11-03 18:13 ` [LTP] " linuxtestproject.agent
  1 sibling, 1 reply; 4+ messages in thread
From: Amir Goldstein @ 2026-11-03 14:44 UTC (permalink / raw)
  To: Edvard Conrad; +Cc: Jan Kara, ltp@lists.linux.it

On Tue, Nov 3, 2026 at 1:14 PM Edvard Conrad <edvard@tuta.io> wrote:
>
> Listing a directory through the event file descriptor makes overlayfs
> open its layers again, after the event fd exists: every layer of a
> merged directory in ovl_dir_read_merged(), and the upper directory of
> an impure one in ovl_dir_read_impure(). Unless those opens inherit
> FMODE_NONOTIFY from the event fd, a listener that marks the backing
> filesystem and lists the directory before it answers is sent a nested
> permission event and blocks in getdents64().
>
> Add a case for each. The impure directory is made by renaming a lower
> file into a directory created on the overlay. The case reports TCONF
> when the overlay cannot record the file's origin and so does not mark
> the directory impure. The impure listing is cached with the inode, so
> the case moves the entry out and back first; otherwise a later
> iteration would read the cache instead of the layer.
>
> Both cases fail on a kernel without the pending overlayfs fix for
> directory opens.
>
> Signed-off-by: Edvard Conrad <edvard@tuta.io>
> ---
> Applies on top of "syscalls/fanotify26: Add directory cases for the
> nested event":
>   https://lore.kernel.org/ltp/P5Iny41--F-9@tuta.io/
>
> Asked for by Amir on the kernel thread, where both paths came up:
>   https://lore.kernel.org/all/CAOQ4uxh67+vs0mr-Lxmwz9-N+F8m29WUNz+8qFLLDpOVYB7=vg@mail.gmail.com/
>
> Tested in a VM on 6b4b543fa18, each kernel run once and with -i 2,
> with the backing filesystem marked:
>
>                                 file  dir   merged dir  impure dir
>                                             listed      listed
>   no fix                        FAIL  FAIL  FAIL        FAIL
>   + c3d355f427ce                PASS  FAIL  FAIL        FAIL
>   + ovl directory fix, v1       PASS  PASS  FAIL        FAIL
>   + ovl directory fix, v2       PASS  PASS  PASS        FAIL
>   + v2 and the follow-up on     PASS  PASS  PASS        PASS
>     the kernel thread
>
> The overlay-only cases pass on all of them. Without the fix the impure
> case fails in both iterations of -i 2, which is what moving the entry
> out and back is for.
>
>  .../kernel/syscalls/fanotify/fanotify26.c     | 92 ++++++++++++++++---
>  1 file changed, 79 insertions(+), 13 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify26.c b/testcases/kernel/syscalls/fanotify/fanotify26.c
> index 3f6b525ef..f6a56821f 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify26.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify26.c
> @@ -23,6 +23,11 @@
>   * backing filesystem as well, which is the shape an anti-malware scanner uses
>   * on an overlayfs root.
>   *
> + * Listing a directory through the event file descriptor can make overlayfs
> + * open its layers again: every layer of a merged directory, and the upper
> + * directory of an impure one, which holds entries moved in from below. The
> + * last two test cases have the listener do that before it answers.
> + *
>   * The test needs root to create permission events and to mount overlayfs.
>   */
>
> @@ -34,7 +39,9 @@
>  #include <signal.h>
>  #include <sys/types.h>
>  #include <sys/wait.h>
> +#include <sys/xattr.h>
>  #include "tst_test.h"
> +#include "lapi/syscalls.h"
>
>  #ifdef HAVE_SYS_FANOTIFY_H
>  #include "fanotify.h"
> @@ -42,10 +49,21 @@
>  #define BACKING_MNT    OVL_BASE_MNTPOINT
>  #define FNAME          "test_file"
>  #define DNAME          "test_dir"
> +#define MNAME          "merged_dir"
> +#define INAME          "impure_dir"
> +#define MOVED          "moved_file"
>  #define OVL_FILE       OVL_MNT "/" FNAME
>  #define OVL_DIR                OVL_MNT "/" DNAME
> +#define OVL_MDIR       OVL_MNT "/" MNAME
> +#define OVL_IDIR       OVL_MNT "/" INAME
> +#define OVL_MOVED      OVL_MNT "/" MOVED
> +#define OVL_IDIR_MOVED OVL_IDIR "/" MOVED
>  #define LOWER_FILE     OVL_LOWER "/" FNAME
>  #define LOWER_DIR      OVL_LOWER "/" DNAME
> +#define LOWER_MDIR     OVL_LOWER "/" MNAME
> +#define LOWER_MOVED    OVL_LOWER "/" MOVED
> +#define UPPER_MDIR     OVL_UPPER "/" MNAME
> +#define UPPER_IDIR     OVL_UPPER "/" INAME
>
>  /*
>   * The listener either answers the event at once or is blocked for good, so
> @@ -56,19 +74,41 @@
>  #define WAIT_MSEC      5000
>
>  static int fd_notify = -1;
> +static int idir_is_impure;
>
>  static struct tcase {
>         const char *tname;
> +       const char *path;
>         int mark_backing;
>         int ondir;
> +       int list;
> +       int impure;
>  } tcases[] = {
> -       {"Overlay mark, file", 0, 0},
> -       {"Overlay and backing mark, file", 1, 0},
> -       {"Overlay mark, directory", 0, 1},
> -       {"Overlay and backing mark, directory", 1, 1},
> +       {"Overlay mark, file", OVL_FILE, 0, 0, 0, 0},
> +       {"Overlay and backing mark, file", OVL_FILE, 1, 0, 0, 0},
> +       {"Overlay mark, directory", OVL_DIR, 0, 1, 0, 0},
> +       {"Overlay and backing mark, directory", OVL_DIR, 1, 1, 0, 0},
> +       {"Overlay and backing mark, merged directory, listed",
> +        OVL_MDIR, 1, 1, 1, 0},
> +       {"Overlay and backing mark, impure directory, listed",
> +        OVL_IDIR, 1, 1, 1, 1},
>  };
>
> -static void answer_one_event(pid_t opener)
> +static void list_dir(int fd)
> +{
> +       char buf[1024];
> +       long ret;
> +
> +       /* Blocks for good if reading a layer generates a nested event. */
> +       do {
> +               ret = tst_syscall(__NR_getdents64, fd, buf, sizeof(buf));
> +       } while (ret > 0);
> +
> +       if (ret == -1)
> +               tst_brk(TBROK | TERRNO, "getdents64() on the event fd failed");
> +}
> +
> +static void answer_one_event(pid_t opener, int list)
>  {
>         struct fanotify_event_metadata event;
>         struct fanotify_response resp;
> @@ -76,6 +116,9 @@ static void answer_one_event(pid_t opener)
>         /* Blocks for good if the event fd generates a nested event. */
>         SAFE_READ(SAFE_READ_ALL, fd_notify, &event, sizeof(event));
>
> +       if (list && event.fd != FAN_NOFD)
> +               list_dir(event.fd);
> +
>         /*
>          * A wrong mask or a stranger's event is a result, not harness
>          * breakage, so report and still answer: the opener stays parked in
> @@ -110,6 +153,20 @@ static void run(unsigned int n)
>
>         tst_res(TINFO, "Test #%d: %s", n, tc->tname);
>
> +       if (tc->impure) {
> +               if (!idir_is_impure) {
> +                       tst_res(TCONF, "Overlay did not mark the directory impure");
> +                       return;
> +               }
> +               /*
> +                * The impure listing is cached with the inode and reused, so
> +                * a later iteration would not open the layer at all. Moving
> +                * an entry with an origin out and back invalidates it.
> +                */
> +               SAFE_RENAME(OVL_IDIR_MOVED, OVL_MOVED);
> +               SAFE_RENAME(OVL_MOVED, OVL_IDIR_MOVED);
> +       }
> +
>         fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
>
>         SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | FAN_MARK_FILESYSTEM,
> @@ -132,10 +189,7 @@ 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.
>                  */
> -               if (tc->ondir)
> -                       fd = open(OVL_DIR, O_RDONLY | O_DIRECTORY);
> -               else
> -                       fd = open(OVL_FILE, O_RDONLY);
> +               fd = open(tc->path, O_RDONLY | (tc->ondir ? O_DIRECTORY : 0));
>                 if (fd != -1)
>                         SAFE_CLOSE(fd);
>                 exit(0);
> @@ -148,7 +202,7 @@ static void run(unsigned int n)
>
>         reader = SAFE_FORK();
>         if (!reader) {
> -               answer_one_event(opener);
> +               answer_one_event(opener, tc->list);
>                 exit(0);
>         }
>
> @@ -160,8 +214,7 @@ static void run(unsigned int n)
>          */
>         if (TST_PROCESS_STATE_WAIT(reader, 'Z',
>                                    tst_multiply_timeout(WAIT_MSEC))) {
> -               tst_res(TFAIL,
> -                       "Listener blocked in read() by a nested event");
> +               tst_res(TFAIL, "Listener blocked by a nested event");
>                 SAFE_KILL(reader, SIGKILL);
>                 SAFE_WAITPID(reader, NULL, 0);
>         }
> @@ -175,12 +228,25 @@ static void run(unsigned int n)
>
>  static void setup(void)
>  {
> -       /* Both have to exist in the lower layer to be opened through it. */
> +       /* These have to exist in the layers to be opened through them. */
>         SAFE_UMOUNT(OVL_MNT);
>         SAFE_TOUCH(LOWER_FILE, 0644, NULL);
>         SAFE_MKDIR(LOWER_DIR, 0755);
> +       SAFE_MKDIR(LOWER_MDIR, 0755);
> +       SAFE_MKDIR(UPPER_MDIR, 0755);
> +       SAFE_TOUCH(LOWER_MOVED, 0644, NULL);
>         SAFE_MOUNT_OVERLAY();
>
> +       /*
> +        * A directory made through the overlay is upper only. Moving a lower
> +        * file into it records the file's origin and marks the directory
> +        * impure, unless the layers cannot carry an origin.
> +        */
> +       SAFE_MKDIR(OVL_IDIR, 0755);
> +       SAFE_RENAME(OVL_MOVED, OVL_IDIR_MOVED);
> +       idir_is_impure =
> +               getxattr(UPPER_IDIR, "trusted.overlay.impure", NULL, 0) != -1;
> +
>         /*
>          * Probe the widest combination run() uses, not a weaker proxy for
>          * it. Every case marks the overlay; the second of each pair also
> --
> 2.56.0
>

Looks good!

Only problem I got:

tst_supported_fs_types.c:67: TINFO: mkfs.ext2 does exist
tst_test.c:1305: TINFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
mke2fs 1.47.2 (1-Jan-2025)
[  107.271105] operation not supported error, dev loop0, sector 614272
op 0x9:(WRITE_ZEROES) flags 0x20800800 phys_seg 0 prio class 2
tst_test.c:1317: TINFO: Mounting /dev/loop0 to
/tmp/LTP_fanfH7dWr/mntpoint fstyp=ext2 flags=0
[  107.359391] ext2 filesystem being mounted at
/tmp/LTP_fanfH7dWr/mntpoint supports timestamps until 2038-01-19
(0x7fffffff)
[  107.367943] overlayfs: upper fs does not support RENAME_WHITEOUT.
tst_test.c:1895: TINFO: Overall timeout per run is 0h 02m 00s
fanotify26.c:232: TINFO: Umounting /tmp/LTP_fanfH7dWr/mntpoint/ovl
[  107.383261] overlayfs: upper fs does not support RENAME_WHITEOUT.
fanotify26.c:246: TBROK: rename(mntpoint/ovl/moved_file,
mntpoint/ovl/impure_dir/moved_file) failed: EINVAL (22)

So I added to the test:

--- a/testcases/kernel/syscalls/fanotify/fanotify26.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify26.c
@@ -275,6 +275,11 @@ static struct tst_test test = {
        .mount_device = 1,
        .needs_overlay = 1,
        .mntpoint = OVL_BASE_MNTPOINT,
+       /* Test requires a base filesystem that supports RENAME_WHITEOUT */
+       .filesystems = (struct tst_fs[]) {
+               {.type = "ext4"},
+               {}
+       },
 };


Other than that:

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [LTP] syscalls/fanotify26: Add cases that list the directory
  2026-11-03 12:14 [LTP] [PATCH] syscalls/fanotify26: Add cases that list the directory Edvard Conrad via ltp
  2026-11-03 14:44 ` Amir Goldstein
@ 2026-11-03 18:13 ` linuxtestproject.agent
  1 sibling, 0 replies; 4+ messages in thread
From: linuxtestproject.agent @ 2026-11-03 18:13 UTC (permalink / raw)
  To: Edvard Conrad; +Cc: ltp

Hi Edvard,

On Tue Nov 3, 2026, Edvard Conrad wrote:
> syscalls/fanotify26: Add cases that list the directory

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

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH] syscalls/fanotify26: Add cases that list the directory
  2026-11-03 14:44 ` Amir Goldstein
@ 2026-11-03 19:44   ` Petr Vorel
  0 siblings, 0 replies; 4+ messages in thread
From: Petr Vorel @ 2026-11-03 19:44 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Jan Kara, ltp@lists.linux.it, Edvard Conrad

Hi Edvard, Amir,

TL;DR: merged, thank you. Few notes below.

> On Tue, Nov 3, 2026 at 1:14 PM Edvard Conrad <edvard@tuta.io> wrote:

> > Listing a directory through the event file descriptor makes overlayfs
> > open its layers again, after the event fd exists: every layer of a
> > merged directory in ovl_dir_read_merged(), and the upper directory of
> > an impure one in ovl_dir_read_impure(). Unless those opens inherit
> > FMODE_NONOTIFY from the event fd, a listener that marks the backing
> > filesystem and lists the directory before it answers is sent a nested
> > permission event and blocks in getdents64().

> > Add a case for each. The impure directory is made by renaming a lower
> > file into a directory created on the overlay. The case reports TCONF
> > when the overlay cannot record the file's origin and so does not mark
"the overlay cannot record the file's origin": I'm just curious how this can
happen. Obviously it's not a mistake when you chose TCONF.

> > the directory impure. The impure listing is cached with the inode, so

> > the case moves the entry out and back first; otherwise a later
> > iteration would read the cache instead of the layer.

> > Both cases fail on a kernel without the pending overlayfs fix for
> > directory opens.

> > Signed-off-by: Edvard Conrad <edvard@tuta.io>
> > ---
> > Applies on top of "syscalls/fanotify26: Add directory cases for the
> > nested event":
> >   https://lore.kernel.org/ltp/P5Iny41--F-9@tuta.io/

> > Asked for by Amir on the kernel thread, where both paths came up:
> >   https://lore.kernel.org/all/CAOQ4uxh67+vs0mr-Lxmwz9-N+F8m29WUNz+8qFLLDpOVYB7=vg@mail.gmail.com/

+1

> > Tested in a VM on 6b4b543fa18, each kernel run once and with -i 2,
> > with the backing filesystem marked:

> >                                 file  dir   merged dir  impure dir
> >                                             listed      listed
> >   no fix                        FAIL  FAIL  FAIL        FAIL
> >   + c3d355f427ce                PASS  FAIL  FAIL        FAIL
> >   + ovl directory fix, v1       PASS  PASS  FAIL        FAIL
> >   + ovl directory fix, v2       PASS  PASS  PASS        FAIL
> >   + v2 and the follow-up on     PASS  PASS  PASS        PASS
> >     the kernel thread

+1. So there will be 2 kernel fixing commits.

> > The overlay-only cases pass on all of them. Without the fix the impure
> > case fails in both iterations of -i 2, which is what moving the entry
> > out and back is for.
...
> >  .../kernel/syscalls/fanotify/fanotify26.c     | 92 ++++++++++++++++---
> >  /*
> >   * The listener either answers the event at once or is blocked for good, so
> > @@ -56,19 +74,41 @@
> >  #define WAIT_MSEC      5000

> >  static int fd_notify = -1;
> > +static int idir_is_impure;

> >  static struct tcase {
> >         const char *tname;
> > +       const char *path;
> >         int mark_backing;
> >         int ondir;
> > +       int list;
> > +       int impure;
> >  } tcases[] = {
> > -       {"Overlay mark, file", 0, 0},
> > -       {"Overlay and backing mark, file", 1, 0},
> > -       {"Overlay mark, directory", 0, 1},
> > -       {"Overlay and backing mark, directory", 1, 1},
> > +       {"Overlay mark, file", OVL_FILE, 0, 0, 0, 0},
> > +       {"Overlay and backing mark, file", OVL_FILE, 1, 0, 0, 0},
> > +       {"Overlay mark, directory", OVL_DIR, 0, 1, 0, 0},
> > +       {"Overlay and backing mark, directory", OVL_DIR, 1, 1, 0, 0},
> > +       {"Overlay and backing mark, merged directory, listed",
> > +        OVL_MDIR, 1, 1, 1, 0},
> > +       {"Overlay and backing mark, impure directory, listed",
> > +        OVL_IDIR, 1, 1, 1, 1},
> >  };

nit: I usually prefer designated initializers instead of list of 0 or 1
(readability), but that's ok.

...
> Looks good!

> Only problem I got:

> tst_supported_fs_types.c:67: TINFO: mkfs.ext2 does exist
> tst_test.c:1305: TINFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
> mke2fs 1.47.2 (1-Jan-2025)
> [  107.271105] operation not supported error, dev loop0, sector 614272
> op 0x9:(WRITE_ZEROES) flags 0x20800800 phys_seg 0 prio class 2
> tst_test.c:1317: TINFO: Mounting /dev/loop0 to
> /tmp/LTP_fanfH7dWr/mntpoint fstyp=ext2 flags=0
> [  107.359391] ext2 filesystem being mounted at
> /tmp/LTP_fanfH7dWr/mntpoint supports timestamps until 2038-01-19
> (0x7fffffff)
> [  107.367943] overlayfs: upper fs does not support RENAME_WHITEOUT.
> tst_test.c:1895: TINFO: Overall timeout per run is 0h 02m 00s
> fanotify26.c:232: TINFO: Umounting /tmp/LTP_fanfH7dWr/mntpoint/ovl
> [  107.383261] overlayfs: upper fs does not support RENAME_WHITEOUT.
> fanotify26.c:246: TBROK: rename(mntpoint/ovl/moved_file,
> mntpoint/ovl/impure_dir/moved_file) failed: EINVAL (22)

> So I added to the test:

> --- a/testcases/kernel/syscalls/fanotify/fanotify26.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify26.c
> @@ -275,6 +275,11 @@ static struct tst_test test = {
>         .mount_device = 1,
>         .needs_overlay = 1,
>         .mntpoint = OVL_BASE_MNTPOINT,
> +       /* Test requires a base filesystem that supports RENAME_WHITEOUT */
> +       .filesystems = (struct tst_fs[]) {
> +               {.type = "ext4"},
> +               {}
> +       },
>  };

+1, I added it before merge.

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-03 19:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03 12:14 [LTP] [PATCH] syscalls/fanotify26: Add cases that list the directory Edvard Conrad via ltp
2026-11-03 14:44 ` Amir Goldstein
2026-11-03 19:44   ` Petr Vorel
2026-11-03 18:13 ` [LTP] " linuxtestproject.agent

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox