| 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
| ||
|
Message-ID: <CACOAw_yzAWpHzHerTtm5vCkejt4Xiex5m_xKMTC_WHspOXFmRg@mail.gmail.com>
Date: Fri, 12 Jan 2024 14:19:44 -0800
From: Daeho Jeong <daeho43@...il.com>
To: Chao Yu <chao@...nel.org>
Cc: jaegeuk@...nel.org, linux-kernel@...r.kernel.org,
linux-f2fs-devel@...ts.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH v4 4/6] f2fs: compress: fix to avoid
inconsistence bewteen i_blocks and dnode
Reviewed-by: Daeho Jeong <daehojeong@...gle.com>
On Wed, Jan 10, 2024 at 10:43 PM Chao Yu <chao@...nel.org> wrote:
>
> In reserve_compress_blocks(), we update blkaddrs of dnode in prior to
> inc_valid_block_count(), it may cause inconsistent status bewteen
> i_blocks and blkaddrs once inc_valid_block_count() fails.
>
> To fix this issue, it needs to reverse their invoking order.
>
> Fixes: c75488fb4d82 ("f2fs: introduce F2FS_IOC_RESERVE_COMPRESS_BLOCKS")
> Signed-off-by: Chao Yu <chao@...nel.org>
> ---
> fs/f2fs/data.c | 5 +++--
> fs/f2fs/f2fs.h | 7 ++++++-
> fs/f2fs/file.c | 26 ++++++++++++++------------
> fs/f2fs/segment.c | 2 +-
> 4 files changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index b171a9980f6a..8d2ace723310 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1219,7 +1219,8 @@ int f2fs_reserve_new_blocks(struct dnode_of_data *dn, blkcnt_t count)
>
> if (unlikely(is_inode_flag_set(dn->inode, FI_NO_ALLOC)))
> return -EPERM;
> - if (unlikely((err = inc_valid_block_count(sbi, dn->inode, &count))))
> + err = inc_valid_block_count(sbi, dn->inode, &count, true);
> + if (unlikely(err))
> return err;
>
> trace_f2fs_reserve_new_blocks(dn->inode, dn->nid,
> @@ -1476,7 +1477,7 @@ static int __allocate_data_block(struct dnode_of_data *dn, int seg_type)
>
> dn->data_blkaddr = f2fs_data_blkaddr(dn);
> if (dn->data_blkaddr == NULL_ADDR) {
> - err = inc_valid_block_count(sbi, dn->inode, &count);
> + err = inc_valid_block_count(sbi, dn->inode, &count, true);
> if (unlikely(err))
> return err;
> }
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 50f3d546ded8..69e71460a950 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2252,7 +2252,7 @@ static inline bool __allow_reserved_blocks(struct f2fs_sb_info *sbi,
>
> static inline void f2fs_i_blocks_write(struct inode *, block_t, bool, bool);
> static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
> - struct inode *inode, blkcnt_t *count)
> + struct inode *inode, blkcnt_t *count, bool partial)
> {
> blkcnt_t diff = 0, release = 0;
> block_t avail_user_block_count;
> @@ -2292,6 +2292,11 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
> avail_user_block_count = 0;
> }
> if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
> + if (!partial) {
> + spin_unlock(&sbi->stat_lock);
> + goto enospc;
> + }
> +
> diff = sbi->total_valid_block_count - avail_user_block_count;
> if (diff > *count)
> diff = *count;
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 80d9c4c096f0..53c495651789 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -3614,14 +3614,16 @@ static int reserve_compress_blocks(struct dnode_of_data *dn, pgoff_t count)
> blkcnt_t reserved;
> int ret;
>
> - for (i = 0; i < cluster_size; i++, dn->ofs_in_node++) {
> - blkaddr = f2fs_data_blkaddr(dn);
> + for (i = 0; i < cluster_size; i++) {
> + blkaddr = data_blkaddr(dn->inode, dn->node_page,
> + dn->ofs_in_node + i);
>
> if (i == 0) {
> - if (blkaddr == COMPRESS_ADDR)
> - continue;
> - dn->ofs_in_node += cluster_size;
> - goto next;
> + if (blkaddr != COMPRESS_ADDR) {
> + dn->ofs_in_node += cluster_size;
> + goto next;
> + }
> + continue;
> }
>
> /*
> @@ -3634,20 +3636,20 @@ static int reserve_compress_blocks(struct dnode_of_data *dn, pgoff_t count)
> compr_blocks++;
> continue;
> }
> -
> - f2fs_set_data_blkaddr(dn, NEW_ADDR);
> }
>
> reserved = cluster_size - compr_blocks;
> if (!reserved)
> goto next;
>
> - ret = inc_valid_block_count(sbi, dn->inode, &reserved);
> - if (ret)
> + ret = inc_valid_block_count(sbi, dn->inode, &reserved, false);
> + if (unlikely(ret))
> return ret;
>
> - if (reserved != cluster_size - compr_blocks)
> - return -ENOSPC;
> + for (i = 0; i < cluster_size; i++, dn->ofs_in_node++) {
> + if (f2fs_data_blkaddr(dn) == NULL_ADDR)
> + f2fs_set_data_blkaddr(dn, NEW_ADDR);
> + }
>
> f2fs_i_compr_blocks_update(dn->inode, compr_blocks, true);
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 4c8836ded90f..ef5b3848426b 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -248,7 +248,7 @@ static int __replace_atomic_write_block(struct inode *inode, pgoff_t index,
> } else {
> blkcnt_t count = 1;
>
> - err = inc_valid_block_count(sbi, inode, &count);
> + err = inc_valid_block_count(sbi, inode, &count, true);
> if (err) {
> f2fs_put_dnode(&dn);
> return err;
> --
> 2.40.1
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@...ts.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Powered by blists - more mailing lists