[PATCH v3] fs: let backing files inherit FMODE_NONOTIFY from the user file

[PATCH v3] fs: let backing files inherit FMODE_NONOTIFY from the user file

[Edvard Conrad]

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: stable@vger.kernel.org
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Edvard Conrad <edvard@tuta.io>
---
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/P50G-yE--F-9@tuta.io/

v2: https://lore.kernel.org/all/P4wV1J9--F-9@tuta.io/
v1: https://lore.kernel.org/all/P4sl5iz--F-9@tuta.io/

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(+)

diff --git a/fs/backing-file.c b/fs/backing-file.c
index 5614cb7801e..24357bf9328 100644
--- a/fs/backing-file.c
+++ b/fs/backing-file.c
@@ -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