fs: let backing files inherit FMODE_NONOTIFY from the user file

When fanotify reports a permission event, copy_event_to_user() creates the event fd with dentry_open_nonotify(), so that the listener's use of that fd does not generate further events. If the event's path is on a stacked filesystem, the stacked filesystem's ->open() opens the real file with backing_file_open(). Since commit ebe559609d78 ("fs: get rid of __FMODE_NONOTIFY kludge"), FMODE_NONOTIFY lives in f_mode only and the backing file does not inherit it: it is allocated with the default fsnotify mode, and fsnotify_open_perm_and_set_mode() raises FS_OPEN_PERM on the real path if the backing filesystem has permission event watchers.

Before that commit __FMODE_NONOTIFY travelled in f_flags, and overlayfs copies the user file's f_flags into the backing open (ovl_open_realfile()), so the backing file was exempt implicitly. Commit bc2473c90fca ("ovl: enable fsnotify events on underlying real files") relied on that when it dropped FMODE_NONOTIFY from OVL_OPEN_FLAGS.

The nested event is queued to the same group, from inside the listener's read(2), and can only be answered by another reader thread of that group. A listener holding filesystem marks on both the stacked and the backing filesystem therefore deadlocks as soon as all of its reader threads are inside such a read(2) at the same time, and every other task that touches a marked filesystem queues up behind it.

Observed with a commercial anti-malware agent (16 reader threads) on an ostree/composefs host: / is an overlayfs whose data layer is the ostree object store on the same btrfs that holds /etc and /home, and a systemd-sysext overlay sits on /usr. Every on-demand scan of /usr wedged the whole machine until the group was killed. Stack of a blocked reader thread on 7.2.7:

fanotify_get_response
fanotify_handle_event
send_to_group
fsnotify
__fsnotify_parent
do_dentry_open
vfs_open
backing_file_open
ovl_open_realfile [overlay]
ovl_open [overlay]
do_dentry_open
vfs_open
backing_file_open
ovl_open_realfile [overlay]
ovl_open [overlay]
do_dentry_open
vfs_open
dentry_open_nonotify
copy_event_to_user
fanotify_read
vfs_read
ksys_read

Userspace cannot avoid this: the backing open happens while the kernel builds the fd, before the event is reported, and fid reporting is not available for permission event classes.

Make a backing file inherit FMODE_NONOTIFY when the user file it is opened for must not generate events. This restores the pre-6.14 behaviour for the fds that fanotify creates (and for any other FMODE_NONOTIFY file), while ordinary opens through a stacked filesystem keep generating events on the real path, as intended by bc2473c90fca.

Verified with a self-contained reproducer (private tmpfs lower layers, an overlayfs over them, FAN_OPEN_PERM filesystem marks on the overlay and on one lower layer, a child opening a file through the overlay) in a qemu VM on this tree built with defconfig plus OVERLAY_FS, FANOTIFY and FANOTIFY_ACCESS_PERMISSIONS: without this patch the listener's read(2) never returns and the child's open is released only by the listener's death; with it read(2) returns, the event is answered and the open completes.

backing_tmpfile_open() is left alone: its only caller is ovl_create_tmpfile(), whose user file is a regular overlayfs file.

Fixes ebe559609d78 ("fs: get rid of __FMODE_NONOTIFY kludge")

Cc [email protected]

Reviewed-by Amir Goldstein <[email protected]>

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

---
v3:
- Back to the v1 change in backing_file_open(). The init_file() variant posted as v2 was larger without being more general; Amir withdrew that suggestion and reviewed v1 instead.

- Add Amir's Reviewed-by. Drop the Assisted-by and Suggested-by trailers and the blank line in the trailer block, as asked.
- Keep the subject line from v2, which is the prefix used for fs/backing-file.c.
- The reproducer posted inline with v1 is now an LTP test, under review as syscalls/fanotify26:
https://lore.kernel.org/ltp/[email protected]/

v2: https://lore.kernel.org/all/[email protected]/
v1: https://lore.kernel.org/all/[email protected]/

Tool disclosure (Documentation/process/generated-content.rst): the stack analysis, the git-history search for the regressing commit, the patch text, the commit message and the reproducer were produced with an AI coding assistant (Anthropic Claude Code) under my direction over interactive sessions; I reviewed, built and tested the result. No static analysis tools beyond checkpatch were used.

 fs/backing-file.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

fs/backing-file.c +11 -1

index 5614cb7801e..24357bf9328 100644
@@ -43,6 +43,17 @@ struct file *backing_file_open(const struct file *user_file, int flags,
        if (IS_ERR(f))
                return f;
 
+       /*
+        * A user file that must not generate fsnotify events (e.g. the fd
+        * that fanotify creates for a permission event with
+        * dentry_open_nonotify()) must not generate them through its backing
+        * file either. Otherwise a listener watching the backing filesystem
+        * gets a nested permission event from its own read(2), which only
+        * another reader of the same group can answer.
+        */
+       if (FMODE_FSNOTIFY_NONE(user_file->f_mode))
+               file_set_fsnotify_mode(f, FMODE_NONOTIFY);
+
        path_get(user_path);
        backing_file_set_user_path(f, user_path);
        error = vfs_open(real_path, f);
--

2.56.0

Make a backing file inherit FMODE_NONOTIFY when the user file it is opened for must not generate events. This restores the pre-6.14 behaviour for the fds that fanotify creates (and for any other FMODE_NONOTIFY file), while ordinary opens through a stacked filesystem keep generating events on the real path, as intended by bc2473c90fca.

Quoted 5 lines · Edvard Conrad

Make a backing file inherit FMODE_NONOTIFY when the user file it is opened for must not generate events. This restores the pre-6.14 behaviour for the fds that fanotify creates (and for any other FMODE_NONOTIFY file), while ordinary opens through a stacked filesystem keep generating events on the real path, as intended by bc2473c90fca.

Looks good to me. I'll take this via vfs.fixes and fix up the subject to use the "backing-file:" prefix while applying.

Amir, note that the same deadlock is still there for directories. ovl_dir_open_realfile() goes through ovl_path_open() -> dentry_open() and not through backing_file_open(). So a FAN_OPEN_PERM|FAN_ONDIR listener that opens an overlay directory through the fd fanotify handed it wedges the same way. That needs to be fixed on the overlayfs side.

Would you mind adding test cases (or variants) to fanotify26 to cover this find and test the attached (sanity tested only) patch.

Quoted 2 lines · Amir Goldstein

Would you mind adding test cases (or variants) to fanotify26 to cover this find and test the attached (sanity tested only) patch.

Two things on the directory side, one settled and one not.

Agreed on 130fdbc3d1f99 ("ovl: pass correct flags for opening real directory"). Before it ovl_dir_open() passed the user file's f_flags to ovl_path_open(), and __FMODE_NONOTIFY travelled with them; the commit reasons through every O_* flag but that one is not an O_* flag. That fits v6.13 failing the directory case and passing the file case.

The one that is not settled: with your patch, listing a merged directory through the event fd still deadlocks. ovl_iterate() sends a directory that has OVL_WHITEOUTS to ovl_iterate_merged(), and ovl_dir_read() opens each layer with plain ovl_path_open(), which no user file reaches. A listener that calls getdents64() on the fd before answering blocks here:

fanotify_handle_event
fsnotify
fsnotify_open_perm_and_set_mode
do_dentry_open
vfs_open
dentry_open
ovl_dir_read_merged
ovl_iterate
iterate_dir
__x64_sys_getdents64

Two controls on the same kernel: the same merged directory without the listing passes, and a lower-only directory with the listing passes, since od->realfile is nonotify with your patch and iterate_dir() uses it directly. That was on 6b4b543fa18 with your patch applied (with some fuzz); the readdir.c paths involved are the same in today's master. I have it as an extra case in a local copy of fanotify26 and can post it once there is a fix to run it against.

Incidentally, I was thinking about your ask for something more
generic: a task-level "creating an event fd" state in fanotify would cover the file and directory opens in one place, and the copy-up opens in copy_up.c too, which also go through plain ovl_path_open() and which an O_RDWR group would trigger for a lower-only file (from reading, not tested). It would not reach the listing case above, which runs at getdents() time, long after the fd exists.

Good catch!
Please test the attached v2 patch and post the new test cases.

Quoted 2 lines · Amir Goldstein

Good catch!
Please test the attached v2 patch and post the new test cases.

v2 fixes the merged directory. An impure directory still deadlocks the same way: ovl_iterate_real() reads it through ovl_cache_get_impure() and ovl_dir_read_impure(), which v2 leaves without the user file, so ovl_dir_read() opens the upper directory with plain ovl_path_open():

fanotify_handle_event
fsnotify
fsnotify_open_perm_and_set_mode
do_dentry_open
vfs_open
dentry_open
ovl_iterate_real
ovl_iterate
iterate_dir
__x64_sys_getdents64

That takes an upper-only directory that a lower file was renamed into, on layers that share a filesystem, so ovl_need_adjust_d_ino() sends it to ovl_iterate_real(). Passing the file down the same way v2 does for the merged case fixes it for me, on top of v2:

fs/overlayfs/readdir.c +5 -3

@@ -670,7 +670,7 @@ static bool ovl_fill_plain(struct dir_context *ctx, const char *name,
 }
 
 static int ovl_dir_read_impure(const struct path *path,  struct list_head *list,
-                              struct rb_root *root)
+                              struct rb_root *root, const struct file *user_file)
 {
        int err;
        struct path realpath;

@@ -678,6 +678,7 @@ static int ovl_dir_read_impure(const struct path *path,  struct list_head *list,
        struct ovl_readdir_data rdd = {
                .ctx.actor = ovl_fill_plain,
                .ctx.count = INT_MAX,
+               .user_file = user_file,
                .list = list,
                .root = root,
        };

@@ -714,9 +715,10 @@ static int ovl_dir_read_impure(const struct path *path,  struct list_head *list,
        return 0;
 }
 
-static struct ovl_dir_cache *ovl_cache_get_impure(const struct path *path)
+static struct ovl_dir_cache *ovl_cache_get_impure(struct file *file)
 {
        int res;
+       const struct path *path = &file->f_path;
        struct dentry *dentry = path->dentry;
        struct inode *inode = d_inode(dentry);
        struct ovl_fs *ofs = OVL_FS(dentry->d_sb);

@@ -735,7 +737,7 @@ static struct ovl_dir_cache *ovl_cache_get_impure(const struct path *path)
                return ERR_PTR(-ENOMEM);
 
        cache->is_refcounted = false;
-       res = ovl_dir_read_impure(path, &cache->entries, &cache->root);
+       res = ovl_dir_read_impure(path, &cache->entries, &cache->root, file);
        if (res) {
                ovl_cache_free(&cache->entries);
                kfree(cache);

Results 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
  + c3d355f427ce                PASS  FAIL  FAIL        FAIL
  + your v1                     PASS  PASS  FAIL        FAIL
  + your v2                     PASS  PASS  PASS        FAIL
  + v2 and the diff above       PASS  PASS  PASS        PASS

The new cases are posted on top of the directory cases:
https://lore.kernel.org/ltp/[email protected]/

Right, I was being sloppy.
Thanks for the fixes.
I added Co-developed-by to attribute your contributions to this fix.

When fanotify reports a permission event, copy_event_to_user() creates the event fd with dentry_open_nonotify(), so that the listener's use of that fd does not generate further events. If the event's path is on a stacked filesystem, the stacked filesystem's ->open() opens the real file with backing_file_open(). Since commit ebe559609d78 ("fs: get rid of __FMODE_NONOTIFY kludge"), FMODE_NONOTIFY lives in f_mode only and the backing file does not inherit it: it is allocated with the default fsnotify mode, and fsnotify_open_perm_and_set_mode() raises FS_OPEN_PERM on the real path if the backing filesystem has permission event watchers.

[...]