[Devel] [PATCH RHEL8 COMMIT] fs/fuse: avoid triggering BUG_ON at file close

[Konstantin Khorenko]

The commit is pushed to "branch-rh8-4.18.0-240.1.1.vz8.5.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh8-4.18.0-240.1.1.vz8.5.22
------>
commit 7b8be41539c610f995aaaf8bcfe8518f564b4a83
Author: Alexey Kuznetsov <kuznet at virtuozzo.com>
Date:   Tue Apr 27 17:08:11 2021 +0300

    fs/fuse: avoid triggering BUG_ON at file close
    
    The problem is that our implementation of vstorage file close
    is not compatible with voluntary taking a reference with fuse file.
    Natural and better fix would be global changing i_ops->fiemap method
    to pass file in addition to inode. Nevertheless, this change is global
    and can be done only in future kernel versions.
    
    Also, replace BUG_ON with WARN_ON. It was not a fatal logical condition,
    kernel can proceed in case we still miss something,
    it is just the places which could violate strict vstorage close semantics.
    
    https://pmc.acronis.com/browse/VSTOR-43056
    Signed-off-by: Alexey Kuznetsov <kuznet at acronis.com>
    
    Cherry-picked from vz7 commit ("386d9fd72108 fs/fuse: avoid triggering BUG_ON at
    file close")
    Signed-off-by: Ildar Ismagilov <ildar.ismagilov at virtuozzo.com>
---
 fs/fuse/file.c   | 27 +++++++++++++--------------
 fs/fuse/fuse_i.h |  3 ++-
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index a25c2d5d2e25..fb55c7968b48 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -386,7 +386,7 @@ void fuse_release_common(struct file *file, bool isdir)
 	 * fuse file release is synchronous
 	 */
 	if (ff->fc->close_wait) {
-		BUG_ON(refcount_read(&ff->count) != 1);
+		WARN_ON(refcount_read(&ff->count) != 1);
 	}
 
 	/*
@@ -413,6 +413,10 @@ static int fuse_release(struct inode *inode, struct file *file)
 	struct fuse_file *ff = file->private_data;
 	struct fuse_inode *fi = get_fuse_inode(inode);
 
+	set_bit(FUSE_S_CLOSING, &ff->ff_state);
+	spin_lock(&fi->lock);
+	spin_unlock(&fi->lock);
+
 	if (ff->fc->writeback_cache) {
 		if (file->f_mode & FMODE_WRITE) {
 			write_inode_now(inode, 1);
@@ -3874,28 +3878,23 @@ static int fuse_request_fiemap(struct inode *inode, u32 cur_max,
 	err = 0;
 	spin_lock(&fi->lock);
 	/* Kernel API is weird in this place, we have to find a file associated with this inode.
-	 * This problem is similar to writepage routines, but it is much worse. When doing
+	 * This problem is similar to writepage routines, but it is worse. When doing
 	 * writepage, we have some file open for write and can take any file from write_files
 	 * and it is safe to keep it with get/put. But here we can have the file open for read
-	 * and no files open for write. Even worse, if we select a random file open for write,
-	 * it can be closed by user from another thread and its close will violate close_wait requirement.
-	 * So, we have to search for a read-only file first and fallback to writable only
-	 * if there is no such file.
+	 * and no files open for write. So, we have to take a file from rw_files.
 	 */
 	if (!list_empty(&fi->rw_files)) {
 		struct fuse_file *t_ff;
 		list_for_each_entry(t_ff, &fi->rw_files, rw_entry) {
-			if (list_empty(&t_ff->write_entry)) {
+			if (!test_bit(FUSE_S_CLOSING, &t_ff->ff_state)) {
 				ff = t_ff;
 				break;
 			}
 		}
-		if (!ff)
-			ff = list_entry(fi->rw_files.next, struct fuse_file, rw_entry);
-		fuse_file_get(ff);
-		inarg.fh = ff->fh;
-	} else if (!list_empty(&fi->write_files)) {
+	}
+	if (!ff && !list_empty(&fi->write_files))
 		ff = list_entry(fi->write_files.next, struct fuse_file, write_entry);
+	if (ff) {
 		fuse_file_get(ff);
 		inarg.fh = ff->fh;
 	} else {
@@ -3917,7 +3916,7 @@ static int fuse_request_fiemap(struct inode *inode, u32 cur_max,
 
 	ap.pages = fuse_pages_alloc(fc->max_pages, GFP_KERNEL, &ap.descs);
 	if (!ap.pages) {
-		fuse_file_put(ff, false, false);
+		fuse_release_ff(inode, ff);
 		return -ENOMEM;
 	}
 
@@ -4001,7 +4000,7 @@ static int fuse_request_fiemap(struct inode *inode, u32 cur_max,
 		__free_page(ap.pages[allocated]);
 		ap.pages[allocated] = NULL;
 	}
-	fuse_file_put(ff, false, false);
+	fuse_release_ff(inode, ff);
 	return err;
 }
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 2d6a7d43a08e..99d871dc641b 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -257,7 +257,8 @@ struct fuse_file {
 /** FUSE file states (ff_state) */
 enum {
 	/** Any fops on given ff should fail immediately */
-	FUSE_S_FAIL_IMMEDIATELY,
+	FUSE_S_FAIL_IMMEDIATELY = 0,
+	FUSE_S_CLOSING		= 1
 };
 
 /** One input argument of a request */