There are multiple calculations and reads of fields of sbi that should
be protected by stat_lock. As stat_lock is not used to read these
values in statfs, this can lead to inconsistent results.
Extend the locking to prevent this issue.
Commit c9c8ed50d94c ("f2fs: fix to avoid potential race on
sbi->unusable_block_count access/update")
already added the use of sbi->stat_lock in statfs in
order to make the calculation of multiple, different fields atomic so
that results are consistent. This is similar to that patch regarding the
change in statfs.

Signed-off-by: Niels Dossche <[email protected]>
---

Changes in v2:
 - protect more fields with locking
 - update commit description

Note:
I am currently working on a static analyser to detect missing locks
using type-based static analysis as my master's thesis
in order to obtain my master's degree.
If you would like to have more details, please let me know.
This was a reported case. I manually verified the report by looking
at the code, so that I do not send wrong information or patches.
After concluding that this seems to be a true positive, I created
this patch. This was compile-tested and runtime-tested on x86_64.
This issue was found on Linux v5.17.4.

 fs/f2fs/super.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index ea939db18f88..a432c80e5e2e 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1731,18 +1731,23 @@ static int f2fs_statfs(struct dentry *dentry, struct 
kstatfs *buf)
        u64 id = huge_encode_dev(sb->s_bdev->bd_dev);
        block_t total_count, user_block_count, start_count;
        u64 avail_node_count;
+       unsigned int total_valid_node_count;
 
        total_count = le64_to_cpu(sbi->raw_super->block_count);
-       user_block_count = sbi->user_block_count;
        start_count = le32_to_cpu(sbi->raw_super->segment0_blkaddr);
        buf->f_type = F2FS_SUPER_MAGIC;
        buf->f_bsize = sbi->blocksize;
 
        buf->f_blocks = total_count - start_count;
+
+       spin_lock(&sbi->stat_lock);
+
+       user_block_count = sbi->user_block_count;
+       total_valid_node_count = valid_node_count(sbi);
+       avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
        buf->f_bfree = user_block_count - valid_user_blocks(sbi) -
                                                sbi->current_reserved_blocks;
 
-       spin_lock(&sbi->stat_lock);
        if (unlikely(buf->f_bfree <= sbi->unusable_block_count))
                buf->f_bfree = 0;
        else
@@ -1755,14 +1760,12 @@ static int f2fs_statfs(struct dentry *dentry, struct 
kstatfs *buf)
        else
                buf->f_bavail = 0;
 
-       avail_node_count = sbi->total_node_count - F2FS_RESERVED_NODE_NUM;
-
        if (avail_node_count > user_block_count) {
                buf->f_files = user_block_count;
                buf->f_ffree = buf->f_bavail;
        } else {
                buf->f_files = avail_node_count;
-               buf->f_ffree = min(avail_node_count - valid_node_count(sbi),
+               buf->f_ffree = min(avail_node_count - total_valid_node_count,
                                        buf->f_bavail);
        }
 
-- 
2.35.2



_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to