Re: [f2fs-dev] [PATCH] f2fs: extend stat_lock to avoid potential race in statfs

[Jaegeuk Kim <jaegeuk@kernel.org>]

On 04/21, Niels Dossche wrote:
> The calculation for f_bfree performs a calculation involving
> current_reserved_blocks and total_valid_user_blocks. Both can be
> modified under stat_lock. As stat_lock is not used to read both 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.

Is this enough? It seems we also need to cover sbi->user_block_count and
sbi->total_node_count by stat_lock.

> 
> Signed-off-by: Niels Dossche <dossche.niels@gmail.com>
> ---
> 
> 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 | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index ea939db18f88..ece768869187 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1739,10 +1739,12 @@ static int f2fs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	buf->f_bsize = sbi->blocksize;
>  
>  	buf->f_blocks = total_count - start_count;
> +
> +	spin_lock(&sbi->stat_lock);
> +
>  	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
> -- 
> 2.35.2


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel