[Devel] [PATCH vz10 02/24] fuse: don't wake page_waitq for non-regular inodes in the invalidate worker

[Pavel Tikhomirov]

On 7/6/26 12:59, Konstantin Khorenko wrote:
> fi->page_waitq lives in an anonymous union in struct fuse_inode that is only valid for regular files; for a directory the same storage holds the readdir cache (fi->rdc, including rdc.lock). fuse_inval_files_work(), handling FUSE_NOTIFY_INVAL_FILES, called wake_up(&fi->page_waitq) for whatever inode the notification targeted -- including directories such as the mount root. Waking page_waitq on a directory walks readdir-cache fields as a wait-queue list.

Commit message formatting is way off, please wrap correctly.

> 
> On a production kernel this stays silent until the directory has been readdir'd (page_waitq.head overlaps rdc.iversion/rdc.lock); once readdir has cached data an INVAL_FILES for that directory walks rdc.iversion as a list pointer -> GPF, with rdc.mtime acting as the 'lock' -> lockup. On a lockdep kernel (64-byte spinlock_t) page_waitq.head.next overlaps rdc.lock.dep_map.name exactly, so it faults on the first directory INVAL_FILES, dereferencing the "&fi->rdc.lock" rodata string as a wait_queue_entry (GPF at a non-canonical ASCII address in the fuse_inval_files_work worker).
> 
> All page_waitq waiters (fuse_release, fuse_wait_on_page_writeback, fuse_set_nowrite) and wakers are regular files, so guard the worker's wake_up() with S_ISREG(). fuse_init_file_inode() already initialises page_waitq for every regular-file inode, so no alloc-time init is needed.
> 
> Fixes: 2604b00b9f44 ("fuse: skip waiting for fuse writeback")
> Feature: vStorage
> https://virtuozzo.atlassian.net/browse/VSTOR-137234
> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
> ---
>  fs/fuse/inode.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index ea158367a1ef..26c0014ccf0f 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -632,7 +632,16 @@ static void fuse_inval_files_work(struct work_struct *w)
>  			fuse_revoke_readpages(ff);
>  		spin_unlock(&fi->lock);
>  
> -		wake_up(&fi->page_waitq); /* readpage[s] can wait on fuse wb */
> +		/*
> +		 * page_waitq lives in a union in struct fuse_inode that is only
> +		 * valid for regular files; on a directory those bytes are the
> +		 * readdir cache (fi->rdc, incl. rdc.lock), so waking it up walks
> +		 * readdir-cache fields as a waitqueue list. All page_waitq
> +		 * waiters (readpage[s] waiting on fuse writeback) are regular
> +		 * files, so only wake it for those.
> +		 */

Comment violates guidance about comments https://docs.kernel.org/process/coding-style.html#commenting

The information about fi->rdc is implementation specific and can easily change, moreover
it's really not that important for understanding the problem. Secondly it's pretty much
obvious from code, that we don't want to use page_waitq for non regular files. Don't think
comment is even needed here, or it should only note that page_waitq is regular file specific.

> +		if (S_ISREG(fi->inode.i_mode))
> +			wake_up(&fi->page_waitq); /* readpage[s] can wait on fuse wb */
>  
>  		truncate_pagecache_range(&fi->inode, 0, -1);
>  		fuse_invalidate_attr(&fi->inode);

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.