linux-kernel - [PATCH 1/5] fuse: check attributes staleness on fuse

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230711043405.66256-2-zhangjiachen.jaycee@bytedance.com>
Date:   Tue, 11 Jul 2023 12:34:01 +0800
From:   Jiachen Zhang <zhangjiachen.jaycee@...edance.com>
To:     Miklos Szeredi <miklos@...redi.hu>,
        Jonathan Corbet <corbet@....net>,
        linux-fsdevel@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     me@...x.top, Jiachen Zhang <zhangjiachen.jaycee@...edance.com>
Subject: [PATCH 1/5] fuse: check attributes staleness on fuse_iget()

Function fuse_direntplus_link() might call fuse_iget() to initialize a new
fuse_inode and change its attributes. If fi->attr_version is always
initialized with 0, even if the attributes returned by the FUSE_READDIR
request is staled, as the new fi->attr_version is 0, fuse_change_attributes
will still set the staled attributes to inode. This wrong behaviour may
cause file size inconsistency even when there is no changes from
server-side.

To reproduce the issue, consider the following 2 programs (A and B) are
running concurrently,

        A                                               B
----------------------------------      --------------------------------
{ /fusemnt/dir/f is a file path in a fuse mount, the size of f is 0. }

readdir(/fusemnt/dir) start
//Daemon set size 0 to f direntry
                                        fallocate(f, 1024)
                                        stat(f) // B see size 1024
                                        echo 2 > /proc/sys/vm/drop_caches
readdir(/fusemnt/dir) reply to kernel
Kernel set 0 to the I_NEW inode

                                        stat(f) // B see size 0

In the above case, only program B is modifying the file size, however, B
observes file size changing between the 2 'readonly' stat() calls. To fix
this issue, we should make sure readdirplus still follows the rule of
attr_version staleness checking even if the fi->attr_version is lost due to
inode eviction. So this patch increases fc->attr_version on inode eviction,
and compares request attr_version and the fc->attr_version when a
FUSE_READDIRPLUS request is finished.

Signed-off-by: Jiachen Zhang <zhangjiachen.jaycee@...edance.com>
---
 fs/fuse/inode.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 660be31aaabc..3e0b1fb1db17 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -115,6 +115,7 @@ static void fuse_free_inode(struct inode *inode)

 static void fuse_evict_inode(struct inode *inode)
 {
+	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_inode *fi = get_fuse_inode(inode);

 	/* Will write inode on close/munmap and in all other dirtiers */
@@ -137,6 +138,8 @@ static void fuse_evict_inode(struct inode *inode)
 		WARN_ON(!list_empty(&fi->write_files));
 		WARN_ON(!list_empty(&fi->queued_writes));
 	}
+
+	atomic64_inc(&fc->attr_version);
 }

 static int fuse_reconfigure(struct fs_context *fsc)
@@ -409,6 +412,10 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
 	fi->nlookup++;
 	spin_unlock(&fi->lock);
 	fuse_change_attributes(inode, attr, attr_valid, attr_version);
+	spin_lock(&fi->lock);
+	if (attr_version < atomic64_read(&fc->attr_version))
+		fuse_invalidate_attr(inode);
+	spin_unlock(&fi->lock);

 	return inode;
 }
-- 
2.20.1