linux-kernel - Re: [PATCH 1/1] f2fs: fix fallocate failed under pinned block situation

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: <c181256e-9f6e-d43e-4d02-a7d8d5286d56@kernel.org>
Date:   Tue, 7 Nov 2023 22:39:35 +0800
From:   Chao Yu <chao@...nel.org>
To:     Wu Bo <bo.wu@...o.com>, Jaegeuk Kim <jaegeuk@...nel.org>
Cc:     linux-f2fs-devel@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org, Wu Bo <wubo.oduw@...il.com>
Subject: Re: [PATCH 1/1] f2fs: fix fallocate failed under pinned block
 situation

On 2023/10/30 17:40, Wu Bo wrote:
> If GC victim has pinned block, it can't be recycled.
> And if GC is foreground running, after many failure try, the pinned file
> is expected to be clear pin flag. To enable the section be recycled.
> 
> But when fallocate trigger FG_GC, GC can never recycle the pinned
> section. Because GC will go to stop before the failure try meet the threshold:
> 	if (has_enough_free_secs(sbi, sec_freed, 0)) {
> 		if (!gc_control->no_bg_gc &&
> 		    total_sec_freed < gc_control->nr_free_secs)
> 			goto go_gc_more;
> 		goto stop;
> 	}
> 
> So when fallocate trigger FG_GC, at least recycle one.

Hmm... it may break pinfile's semantics at least on one pinned file?
In this case, I prefer to fail fallocate() rather than unpinning file,
in order to avoid leaving invalid LBA references of unpinned file held
by userspace.

Thoughts?

Thanks,

> 
> This issue can be reproduced by filling f2fs space as following layout.
> Every segment has one block is pinned:
> +-+-+-+-+-+-+-----+-+
> | | |p| | | | ... | | seg_n
> +-+-+-+-+-+-+-----+-+
> +-+-+-+-+-+-+-----+-+
> | | |p| | | | ... | | seg_n+1
> +-+-+-+-+-+-+-----+-+
> ...
> +-+-+-+-+-+-+-----+-+
> | | |p| | | | ... | | seg_n+k
> +-+-+-+-+-+-+-----+-+
> 
> And following are steps to reproduce this issue:
> dd if=/dev/zero of=./f2fs_pin.img bs=2M count=1024
> mkfs.f2fs f2fs_pin.img
> mkdir f2fs
> mount f2fs_pin.img ./f2fs
> cd f2fs
> dd if=/dev/zero of=./large_padding bs=1M count=1760
> ./pin_filling.sh
> rm padding*
> sync
> touch fallocate_40m
> f2fs_io pinfile set fallocate_40m
> fallocate -l 41943040 fallocate_40m
> 
> fallocate always fail with EAGAIN even there has enough free space.
> 
> 'pin_filling.sh' is:
> count=1
> while :
> do
>      # filling the seg space
>      for i in {1..511}:
>      do
>          name=padding_$count-$i
>          echo write $name
>          dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1
>          if [ $? -ne 0 ]; then
>                  exit 0
>          fi
>      done
>      sync
> 
>      # pin one block in a segment
>      name=pin_file$count
>      dd if=/dev/zero of=./$name bs=4K count=1 > /dev/null 2>&1
>      sync
>      f2fs_io pinfile set $name
>      count=$(($count + 1))
> done
> 
> Signed-off-by: Wu Bo <bo.wu@...o.com>
> ---
>   fs/f2fs/file.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index ca5904129b16..e8a13616543f 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1690,7 +1690,7 @@ static int f2fs_expand_inode_data(struct inode *inode, loff_t offset,
>   			.init_gc_type = FG_GC,
>   			.should_migrate_blocks = false,
>   			.err_gc_skipped = true,
> -			.nr_free_secs = 0 };
> +			.nr_free_secs = 1 };
>   	pgoff_t pg_start, pg_end;
>   	loff_t new_size;
>   	loff_t off_end;