2022-07-18 11:25:26

by yebin (H)

[permalink] [raw]
Subject: [PATCH -next RFC] f2fs: fix null-ptr-deref in f2fs_get_dnode_of_data

There is issue as follows when test f2fs atomic write:
F2FS-fs (loop0): Can't find valid F2FS filesystem in 2th superblock
F2FS-fs (loop0): invalid crc_offset: 0
F2FS-fs (loop0): f2fs_check_nid_range: out-of-range nid=1, run fsck to fix.
F2FS-fs (loop0): f2fs_check_nid_range: out-of-range nid=2, run fsck to fix.
==================================================================
BUG: KASAN: null-ptr-deref in f2fs_get_dnode_of_data+0xac/0x16d0
Read of size 8 at addr 0000000000000028 by task rep/1990

CPU: 4 PID: 1990 Comm: rep Not tainted 5.19.0-rc6-next-20220715 #266
Call Trace:
<TASK>
dump_stack_lvl+0x6e/0x91
print_report.cold+0x49a/0x6bb
kasan_report+0xa8/0x130
f2fs_get_dnode_of_data+0xac/0x16d0
f2fs_do_write_data_page+0x2a5/0x1030
move_data_page+0x3c5/0xdf0
do_garbage_collect+0x2015/0x36c0
f2fs_gc+0x554/0x1d30
f2fs_balance_fs+0x7f5/0xda0
f2fs_write_single_data_page+0xb66/0xdc0
f2fs_write_cache_pages+0x716/0x1420
f2fs_write_data_pages+0x84f/0x9a0
do_writepages+0x130/0x3a0
filemap_fdatawrite_wbc+0x87/0xa0
file_write_and_wait_range+0x157/0x1c0
f2fs_do_sync_file+0x206/0x12d0
f2fs_sync_file+0x99/0xc0
vfs_fsync_range+0x75/0x140
f2fs_file_write_iter+0xd7b/0x1850
vfs_write+0x645/0x780
ksys_write+0xf1/0x1e0
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x63/0xcd

As 3db1de0e582c commit changed atomic write way which new a cow_inode for
atomic write file, and also mark cow_inode as FI_ATOMIC_FILE.
When f2fs_do_write_data_page write cow_inode will use cow_inode's cow_inode
which is NULL. Then will trigger null-ptr-deref.
To solve above issue, don't mark cow_inode as FI_ATOMIC_FILE.

Fiexes: 3db1de0e582c("f2fs: change the current atomic write way")
Signed-off-by: Ye Bin <[email protected]>
---
fs/f2fs/file.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 466eb00aa516..735802b6b754 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2062,7 +2062,6 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
spin_unlock(&sbi->inode_lock[ATOMIC_FILE]);

set_inode_flag(inode, FI_ATOMIC_FILE);
- set_inode_flag(fi->cow_inode, FI_ATOMIC_FILE);
clear_inode_flag(fi->cow_inode, FI_INLINE_DATA);
f2fs_up_write(&fi->i_gc_rwsem[WRITE]);

--
2.31.1


2022-07-24 10:26:24

by Chao Yu

[permalink] [raw]
Subject: Re: [PATCH -next RFC] f2fs: fix null-ptr-deref in f2fs_get_dnode_of_data

On 2022/7/18 19:36, Ye Bin wrote:
> There is issue as follows when test f2fs atomic write:
> F2FS-fs (loop0): Can't find valid F2FS filesystem in 2th superblock
> F2FS-fs (loop0): invalid crc_offset: 0
> F2FS-fs (loop0): f2fs_check_nid_range: out-of-range nid=1, run fsck to fix.
> F2FS-fs (loop0): f2fs_check_nid_range: out-of-range nid=2, run fsck to fix.
> ==================================================================
> BUG: KASAN: null-ptr-deref in f2fs_get_dnode_of_data+0xac/0x16d0
> Read of size 8 at addr 0000000000000028 by task rep/1990
>
> CPU: 4 PID: 1990 Comm: rep Not tainted 5.19.0-rc6-next-20220715 #266
> Call Trace:
> <TASK>
> dump_stack_lvl+0x6e/0x91
> print_report.cold+0x49a/0x6bb
> kasan_report+0xa8/0x130
> f2fs_get_dnode_of_data+0xac/0x16d0
> f2fs_do_write_data_page+0x2a5/0x1030
> move_data_page+0x3c5/0xdf0
> do_garbage_collect+0x2015/0x36c0
> f2fs_gc+0x554/0x1d30
> f2fs_balance_fs+0x7f5/0xda0
> f2fs_write_single_data_page+0xb66/0xdc0
> f2fs_write_cache_pages+0x716/0x1420
> f2fs_write_data_pages+0x84f/0x9a0
> do_writepages+0x130/0x3a0
> filemap_fdatawrite_wbc+0x87/0xa0
> file_write_and_wait_range+0x157/0x1c0
> f2fs_do_sync_file+0x206/0x12d0
> f2fs_sync_file+0x99/0xc0
> vfs_fsync_range+0x75/0x140
> f2fs_file_write_iter+0xd7b/0x1850
> vfs_write+0x645/0x780
> ksys_write+0xf1/0x1e0
> do_syscall_64+0x3b/0x90
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
> As 3db1de0e582c commit changed atomic write way which new a cow_inode for
> atomic write file, and also mark cow_inode as FI_ATOMIC_FILE.
> When f2fs_do_write_data_page write cow_inode will use cow_inode's cow_inode
> which is NULL. Then will trigger null-ptr-deref.
> To solve above issue, don't mark cow_inode as FI_ATOMIC_FILE.
>
> Fiexes: 3db1de0e582c("f2fs: change the current atomic write way")
> Signed-off-by: Ye Bin <[email protected]>
> ---
> fs/f2fs/file.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 466eb00aa516..735802b6b754 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -2062,7 +2062,6 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
> spin_unlock(&sbi->inode_lock[ATOMIC_FILE]);
>
> set_inode_flag(inode, FI_ATOMIC_FILE);
> - set_inode_flag(fi->cow_inode, FI_ATOMIC_FILE);

It needs to remove clear_inode_flag(fi->cow_inode, FI_ATOMIC_FILE) in
f2fs_abort_atomic_write()?

To Daeho, could you please review this patch? IIUC, cow_inode doesn't
need to be set as atomic file?

Thanks,

> clear_inode_flag(fi->cow_inode, FI_INLINE_DATA);
> f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
>

2022-07-29 18:19:17

by Daeho Jeong

[permalink] [raw]
Subject: Re: [PATCH -next RFC] f2fs: fix null-ptr-deref in f2fs_get_dnode_of_data

On Sun, Jul 24, 2022 at 3:21 AM Chao Yu <[email protected]> wrote:
>
> On 2022/7/18 19:36, Ye Bin wrote:
> > There is issue as follows when test f2fs atomic write:
> > F2FS-fs (loop0): Can't find valid F2FS filesystem in 2th superblock
> > F2FS-fs (loop0): invalid crc_offset: 0
> > F2FS-fs (loop0): f2fs_check_nid_range: out-of-range nid=1, run fsck to fix.
> > F2FS-fs (loop0): f2fs_check_nid_range: out-of-range nid=2, run fsck to fix.
> > ==================================================================
> > BUG: KASAN: null-ptr-deref in f2fs_get_dnode_of_data+0xac/0x16d0
> > Read of size 8 at addr 0000000000000028 by task rep/1990
> >
> > CPU: 4 PID: 1990 Comm: rep Not tainted 5.19.0-rc6-next-20220715 #266
> > Call Trace:
> > <TASK>
> > dump_stack_lvl+0x6e/0x91
> > print_report.cold+0x49a/0x6bb
> > kasan_report+0xa8/0x130
> > f2fs_get_dnode_of_data+0xac/0x16d0
> > f2fs_do_write_data_page+0x2a5/0x1030
> > move_data_page+0x3c5/0xdf0
> > do_garbage_collect+0x2015/0x36c0
> > f2fs_gc+0x554/0x1d30
> > f2fs_balance_fs+0x7f5/0xda0
> > f2fs_write_single_data_page+0xb66/0xdc0
> > f2fs_write_cache_pages+0x716/0x1420
> > f2fs_write_data_pages+0x84f/0x9a0
> > do_writepages+0x130/0x3a0
> > filemap_fdatawrite_wbc+0x87/0xa0
> > file_write_and_wait_range+0x157/0x1c0
> > f2fs_do_sync_file+0x206/0x12d0
> > f2fs_sync_file+0x99/0xc0
> > vfs_fsync_range+0x75/0x140
> > f2fs_file_write_iter+0xd7b/0x1850
> > vfs_write+0x645/0x780
> > ksys_write+0xf1/0x1e0
> > do_syscall_64+0x3b/0x90
> > entry_SYSCALL_64_after_hwframe+0x63/0xcd
> >
> > As 3db1de0e582c commit changed atomic write way which new a cow_inode for
> > atomic write file, and also mark cow_inode as FI_ATOMIC_FILE.
> > When f2fs_do_write_data_page write cow_inode will use cow_inode's cow_inode
> > which is NULL. Then will trigger null-ptr-deref.
> > To solve above issue, don't mark cow_inode as FI_ATOMIC_FILE.
> >
> > Fiexes: 3db1de0e582c("f2fs: change the current atomic write way")
> > Signed-off-by: Ye Bin <[email protected]>
> > ---
> > fs/f2fs/file.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index 466eb00aa516..735802b6b754 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -2062,7 +2062,6 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
> > spin_unlock(&sbi->inode_lock[ATOMIC_FILE]);
> >
> > set_inode_flag(inode, FI_ATOMIC_FILE);
> > - set_inode_flag(fi->cow_inode, FI_ATOMIC_FILE);
>
> It needs to remove clear_inode_flag(fi->cow_inode, FI_ATOMIC_FILE) in
> f2fs_abort_atomic_write()?
>
> To Daeho, could you please review this patch? IIUC, cow_inode doesn't
> need to be set as atomic file?
>

Sorry for the late reply, I was OOO.

I think I missed the FG GC case for COW inode. :(
I used the FI_ATOMIC_FILE flag for COW inode in
__get_segment_type_6(). Maybe we need another flag for the COW inode
case in the function, if we remove the flag from the COW inode.
Plus, we need to remove clear_inode_flag(fi->cow_inode,
FI_ATOMIC_FILE) too in f2fs_abort_atomic_write().

Thanks,

> Thanks,
>
> > clear_inode_flag(fi->cow_inode, FI_INLINE_DATA);
> > f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
> >