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

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

[Konstantin Khorenko]

FUSE_NOTIFY_INVAL_FILES invalidates the read/write io state of an inode,
which only regular files have.  The notification carries a 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: a GPF in the worker, or a lockup
on a bogus lock, depending on what the overlapping fields happen to hold.

vstorage never sends the notification for anything but a regular file,
but nothing in the protocol or in the kernel makes that a rule: the
message carries a bare nodeid, and 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.  That is how it was found: a
selftest driving /dev/fuse by hand picked FUSE_ROOT_ID as the one nodeid
guaranteed to exist without a preceding LOOKUP, and the notification for
it crashed the kernel.  A fuse server can be started by an unprivileged
user, so this is reachable without any privileges.

Fixes: eab013c7511c ("fuse: skip waiting for fuse writeback")
Feature: vStorage
https://virtuozzo.atlassian.net/browse/VSTOR-137234
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
v1 -> v2:
 - reject the notification in fuse_invalidate_files() instead of merely
   skipping the wake_up() in fuse_inval_files_work(): a non-regular inode
   is not a valid target for FUSE_NOTIFY_INVAL_FILES at all, so fail it
   with -EOPNOTSUPP (dropping the reference taken by fuse_ilookup())
 - shrink the code comment down to why fi->page_waitq must not be touched
   for non-regular inodes, the struct fuse_inode layout details are
   implementation specific and moved to the commit message
 - reword the commit message: spell out the call path, which inode type
   initialises what, and how a non-regular nodeid reaches the worker

 fs/fuse/inode.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index ea158367a1efd..3ca447cb3b6eb 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);
 
 	/*
-- 
2.43.0

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

[Konstantin Khorenko]

a kind ping

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 8/19/26 18:05, Konstantin Khorenko wrote:
> FUSE_NOTIFY_INVAL_FILES invalidates the read/write io state of an inode,
> which only regular files have.  The notification carries a 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: a GPF in the worker, or a lockup
> on a bogus lock, depending on what the overlapping fields happen to hold.
> 
> vstorage never sends the notification for anything but a regular file,
> but nothing in the protocol or in the kernel makes that a rule: the
> message carries a bare nodeid, and 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.  That is how it was found: a
> selftest driving /dev/fuse by hand picked FUSE_ROOT_ID as the one nodeid
> guaranteed to exist without a preceding LOOKUP, and the notification for
> it crashed the kernel.  A fuse server can be started by an unprivileged
> user, so this is reachable without any privileges.
> 
> Fixes: eab013c7511c ("fuse: skip waiting for fuse writeback")
> Feature: vStorage
> https://virtuozzo.atlassian.net/browse/VSTOR-137234
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> ---
> v1 -> v2:
>  - reject the notification in fuse_invalidate_files() instead of merely
>    skipping the wake_up() in fuse_inval_files_work(): a non-regular inode
>    is not a valid target for FUSE_NOTIFY_INVAL_FILES at all, so fail it
>    with -EOPNOTSUPP (dropping the reference taken by fuse_ilookup())
>  - shrink the code comment down to why fi->page_waitq must not be touched
>    for non-regular inodes, the struct fuse_inode layout details are
>    implementation specific and moved to the commit message
>  - reword the commit message: spell out the call path, which inode type
>    initialises what, and how a non-regular nodeid reaches the worker
> 
>  fs/fuse/inode.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index ea158367a1efd..3ca447cb3b6eb 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);
>  
>  	/*

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