[Devel] [PATCH vz10 v3] fuse: reject FUSE_NOTIFY_INVAL_FILES for non-regular inodes

[Konstantin Khorenko <khorenko@virtuozzo.com>]

FUSE_NOTIFY_INVAL_FILES invalidates the read/write io state of an inode,
which only regular files have. The notification carries a bare nodeid,
and the actual work is deferred to a workqueue:

  fuse_dev_do_write()
   fuse_notify()
    fuse_notify_inval_files()
     fuse_invalidate_files()          inode = fuse_ilookup(fc, nodeid)
      queue_delayed_work(fuse_inval_files_wq, &fc->inval_files_work)
       ...
       fuse_inval_files_work()        worker context
        wake_up(&fi->page_waitq)      regular file state

What the bytes behind fi->page_waitq are depends on the inode type, see
fuse_init_inode():

  S_ISREG  fuse_init_file_inode()  init_waitqueue_head(&fi->page_waitq)
  S_ISDIR  fuse_init_dir()         the same union holds fi->rdc, the
                                   readdir cache (rdc.lock, rdc.version,
                                   rdc.mtime, ...)
  other    -                       nothing, fuse_alloc_inode() leaves
                                   the union as allocated

So for a nodeid which is not a regular file - the mount root, say - the
worker calls wake_up() on storage that is not a waitqueue and walks
whatever lies there as a waitqueue list: an oops in the worker, or a
lockup on a bogus lock, depending on what the overlapping fields happen
to hold.

The notification is only accepted on a close_wait mount, so the sender
is vstorage, and vstorage never names anything but a regular file.
Nothing in the protocol or in the kernel makes that a rule though: every
nodeid the server has handed out is a candidate, FUSE_ROOT_ID included -
the mount root is always a directory and always in the inode cache.

https://virtuozzo.atlassian.net/browse/VSTOR-147641

Fixes: 19dd5fa442ea ("fuse: skip waiting for fuse writeback")
Feature: vStorage
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
FUSE_NOTIFY_INVAL_FILES carries a bare nodeid, and the kernel serves it
for whatever inode that nodeid resolves to. Only a regular file has the
io state the notification invalidates: for a directory the same bytes
hold the readdir cache, for a symlink or a device node they hold
whatever the slab handed out. The deferred worker wakes them as a wait
queue all the same.

The reproducer below keeps /dev/fuse to itself, answers the FUSE_INIT
handshake by hand and sends one notification for FUSE_ROOT_ID - the
mount root, always a directory and always resolvable without a
preceding LOOKUP. It wants root in ve0: since ae86620879381 ("fs/fuse:
strictly segregate vstorage features of generic fuse") the notification
is only accepted on a close_wait mount, and close_wait takes ve0 root
with CAP_SYS_ADMIN (is_close_wait_allowed()). Unpatched, the
notification is accepted and the fuse_inval_files_work() worker dies on
it a moment later; patched, the write(2) returns EOPNOTSUPP and nothing
is queued.

  // SPDX-License-Identifier: GPL-2.0
  /*
   * FUSE_NOTIFY_INVAL_FILES for a directory nodeid. Run as root in ve0.
   * Unfixed kernel: the fuse_inval_files_work() worker dies on the
   * notification. Fixed kernel: the write(2) fails with EOPNOTSUPP.
   */
  #define _GNU_SOURCE
  #include <sys/mount.h>
  #include <sched.h>
  #include <unistd.h>
  #include <stdio.h>
  #include <stdlib.h>
  #include <string.h>
  #include <errno.h>
  #include <fcntl.h>
  #include <stdint.h>
  #include <linux/fuse.h>

  #ifndef FUSE_NOTIFY_INVAL_FILES
  #define FUSE_NOTIFY_INVAL_FILES 77
  struct fuse_notify_inval_files_out {
  	uint64_t	ino;
  };
  #endif

  static int answer_init(int fd)
  {
  	char buf[FUSE_MIN_READ_BUFFER];
  	struct fuse_in_header *inh = (void *)buf;
  	struct fuse_init_in *in;
  	struct {
  		struct fuse_out_header oh;
  		struct fuse_init_out arg;
  	} __attribute__((packed)) rep;
  	ssize_t n;

  	do {
  		n = read(fd, buf, sizeof(buf));
  		if (n < (ssize_t)(sizeof(*inh) + sizeof(*in)))
  			return -1;
  	} while (inh->opcode != FUSE_INIT);

  	in = (void *)(buf + sizeof(*inh));
  	memset(&rep, 0, sizeof(rep));
  	rep.oh.len = sizeof(rep);
  	rep.oh.unique = inh->unique;
  	rep.arg.major = FUSE_KERNEL_VERSION;
  	rep.arg.minor = FUSE_KERNEL_MINOR_VERSION;
  	rep.arg.max_readahead = in->max_readahead;
  	/* fuse_invalidate_files() refuses to run without async_read */
  	rep.arg.flags = FUSE_ASYNC_READ;
  	rep.arg.max_write = 1 << 20;

  	return write(fd, &rep, sizeof(rep)) == (ssize_t)sizeof(rep) ? 0 : -1;
  }

  int main(void)
  {
  	struct {
  		struct fuse_out_header oh;
  		struct fuse_notify_inval_files_out arg;
  	} __attribute__((packed)) msg;
  	char mnt[] = "/tmp/fuse_inval_XXXXXX";
  	char opts[128];
  	int fd;

  	fd = open("/dev/fuse", O_RDWR);
  	if (fd < 0) {
  		perror("/dev/fuse");
  		return 1;
  	}
  	if (unshare(CLONE_NEWNS) ||
  	    mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL) ||
  	    !mkdtemp(mnt)) {
  		perror("mount namespace");
  		return 1;
  	}

  	snprintf(opts, sizeof(opts),
  		 "fd=%d,rootmode=40755,user_id=0,group_id=0,close_wait", fd);
  	if (mount("repro", mnt, "fuse", 0, opts)) {
  		/* close_wait takes CAP_SYS_ADMIN in ve0 */
  		perror("mount");
  		return 1;
  	}
  	if (answer_init(fd)) {
  		fprintf(stderr, "FUSE_INIT handshake failed\n");
  		return 1;
  	}

  	memset(&msg, 0, sizeof(msg));
  	msg.oh.len = sizeof(msg);
  	msg.oh.error = FUSE_NOTIFY_INVAL_FILES;
  	msg.oh.unique = 0;
  	msg.arg.ino = FUSE_ROOT_ID;

  	if (write(fd, &msg, sizeof(msg)) < 0)
  		perror("FUSE_NOTIFY_INVAL_FILES");
  	else
  		printf("notification accepted\n");

  	return 0;
  }

That close_wait gate is also why the selftest on the selftests branch,
b8d769b55b3b ("selftests/ve: add fuse invalidate_files notification
selftest"), stops reaching this code once it is rebased onto a branch
that has ae86620879381: it mounts a plain fuse filesystem, so every
notification it writes fails with EINVAL in fuse_notify_inval_files()
before fuse_invalidate_files() is called. It needs the close_wait mount
option to keep testing anything, and its
root_inode_is_a_directory_and_is_accepted case has to expect EOPNOTSUPP
once this patch is in.

That series also carries an alternative fix for this crash,
2d489f8b4c05 ("fuse: don't wake page_waitq for non-regular inodes in
the invalidate worker"), which keeps the notification successful and
only skips the wake_up() in the worker. Only one of the two should
land. Refusing the notification outright also keeps the worker from
truncating the page cache of a directory - that is the readdir cache -
for a notification that means nothing for a directory.

Worth a word from vstorage before this is applied: on a close_wait
mount fuse_notify() refuses every notification code but INVAL_FILES, so
an INVAL_FILES for a directory is the only handle the daemon has on a
directory's page cache today. If anything leans on that, the worker
side guard is the fix to take instead.
---
Changes in v3:
- fix the Fixes: tag. eab013c7511c is the hash "fuse: skip waiting for
  fuse writeback" has on branch-rh10-6.12.0-211.39.1.16.x.vz10-ovz; the
  rebase to 211.52.1.22 renumbered it to 19dd5fa442ea
- drop the claim that an unprivileged fuse server can trigger this: the
  notification now takes a close_wait mount, so the sender is vstorage
- drop the account of how the bug was found
- add the reproducer and the note about the selftest above
- Link to v2: http://lore.virtuozzo.com/20260819160529.490608-1-khorenko@virtuozzo.com
- Link to v1: http://lore.virtuozzo.com/20260706110002.1024515-3-khorenko@virtuozzo.com

The patch itself is unchanged since v2.
---
 fs/fuse/inode.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index eb2e9a4f87564..025984980c65f 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -667,6 +667,16 @@ int fuse_invalidate_files(struct fuse_dev *fud, u64 nodeid)
 	if (!inode)
 		return -ENOENT;
 
+	/*
+	 * fuse_inval_files_work() wakes fi->page_waitq, which is initialised
+	 * for regular files only - for other inode types it is not a
+	 * waitqueue at all.
+	 */
+	if (!S_ISREG(inode->i_mode)) {
+		iput(inode);
+		return -EOPNOTSUPP;
+	}
+
 	fi = get_fuse_inode(inode);
 
 	/*

---
base-commit: ca54f152160587a74ff17cb52feef16f7b9d4e2b
change-id: 20260928-b4-prep-kh-vstor-147641-fuse_notify_inval_files-213ca0da419a

Best regards,
--  
Konstantin Khorenko <khorenko@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel