Re: Patch "f2fs: fix to data block override node segment by mistake" has been added to the 4.19-stable tree

From: Greg KH
Date: Tue Dec 03 2019 - 17:05:56 EST


On Sun, Dec 01, 2019 at 10:15:19AM -0500, Sasha Levin wrote:
> This is a note to let you know that I've just added the patch titled
>
> f2fs: fix to data block override node segment by mistake
>
> to the 4.19-stable tree which can be found at:
> http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>
> The filename of the patch is:
> f2fs-fix-to-data-block-override-node-segment-by-mist.patch
> and it can be found in the queue-4.19 subdirectory.
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@xxxxxxxxxxxxxxx> know about it.
>
>
>
> commit 9db064cac1f087e2f67fcd2b60b33a8047fe4294
> Author: zhengliang <zhengliang6@xxxxxxxxxx>
> Date: Mon Mar 4 09:32:25 2019 +0800
>
> f2fs: fix to data block override node segment by mistake
>
> [ Upstream commit a0770e13c8da83bdb64738c0209ab02dd3cfff8b ]
>
> v4: Rearrange the previous three versions.
>
> The following scenario could lead to data block override by mistake.
>
> TASK A | TASK kworker | TASK B | TASK C
> | | |
> open | | |
> write | | |
> close | | |
> | f2fs_write_data_pages | |
> | f2fs_write_cache_pages | |
> | f2fs_outplace_write_data | |
> | f2fs_allocate_data_block (get block in seg S, | |
> | S is full, and only | |
> | have this valid data | |
> | block) | |
> | allocate_segment | |
> | locate_dirty_segment (mark S as PRE) | |
> | f2fs_submit_page_write (submit but is not | |
> | written on dev) | |
> unlink | | |
> iput_final | | |
> f2fs_drop_inode | | |
> f2fs_truncate | | |
> (not evict) | | |
> | | write_checkpoint |
> | | flush merged bio but not wait file data writeback |
> | | set_prefree_as_free (mark S as FREE) |
> | | | update NODE/DATA
> | | | allocate_segment (select S)
> | writeback done | |
>
> So we need to guarantee io complete before truncate inode in f2fs_drop_inode.
>
> Reviewed-by: Chao Yu <yuchao0@xxxxxxxxxx>
> Signed-off-by: Zheng Liang <zhengliang6@xxxxxxxxxx>
> Signed-off-by: Jaegeuk Kim <jaegeuk@xxxxxxxxxx>
> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 7a9cc64f5ca37..b82b7163e0d08 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -890,6 +890,10 @@ static int f2fs_drop_inode(struct inode *inode)
> sb_start_intwrite(inode->i_sb);
> f2fs_i_size_write(inode, 0);
>
> + f2fs_submit_merged_write_cond(F2FS_I_SB(inode),
> + inode, NULL, 0, DATA);
> + truncate_inode_pages_final(inode->i_mapping);
> +
> if (F2FS_HAS_BLOCKS(inode))
> f2fs_truncate(inode);
>

This adds a build warning, which I think implies that this backport is
not correct:

CC [M] fs/f2fs/super.o
In file included from ./include/uapi/linux/posix_types.h:5,
from ./include/uapi/linux/types.h:14,
from ./include/linux/types.h:6,
from ./include/linux/list.h:5,
from ./include/linux/module.h:9,
from fs/f2fs/super.c:11:
fs/f2fs/super.c: In function âf2fs_drop_inodeâ:
./include/linux/stddef.h:8:14: warning: passing argument 3 of âf2fs_submit_merged_write_condâ makes integer from pointer without a cast [-Wint-conversion]
8 | #define NULL ((void *)0)
| ^~~~~~~~~~~
| |
| void *
fs/f2fs/super.c:894:13: note: in expansion of macro âNULLâ
894 | inode, NULL, 0, DATA);
| ^~~~
In file included from fs/f2fs/super.c:30:
fs/f2fs/f2fs.h:3037:32: note: expected ânid_tâ {aka âunsigned intâ} but argument is of type âvoid *â
3037 | struct inode *inode, nid_t ino, pgoff_t idx,
| ~~~~~~^~~

I'm going to drop this patch from the 4.19 tree because of this.

thanks,

greg k-h