Re: [PATCH] f2fs: fix to avoid use-after-free issue in f2fs_filemap_fault

[Jaegeuk Kim]

On 03/14, Chao Yu wrote:
> syzbot reports a f2fs bug as below:
> 
> BUG: KASAN: slab-use-after-free in f2fs_filemap_fault+0xd1/0x2c0 fs/f2fs/file.c:49
> Read of size 8 at addr ffff88807bb22680 by task syz-executor184/5058
> 
> CPU: 0 PID: 5058 Comm: syz-executor184 Not tainted 6.7.0-syzkaller-09928-g052d534373b7 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 11/17/2023
> Call Trace:
>  <TASK>
>  __dump_stack lib/dump_stack.c:88 [inline]
>  dump_stack_lvl+0x1e7/0x2d0 lib/dump_stack.c:106
>  print_address_description mm/kasan/report.c:377 [inline]
>  print_report+0x163/0x540 mm/kasan/report.c:488
>  kasan_report+0x142/0x170 mm/kasan/report.c:601
>  f2fs_filemap_fault+0xd1/0x2c0 fs/f2fs/file.c:49
>  __do_fault+0x131/0x450 mm/memory.c:4376
>  do_shared_fault mm/memory.c:4798 [inline]
>  do_fault mm/memory.c:4872 [inline]
>  do_pte_missing mm/memory.c:3745 [inline]
>  handle_pte_fault mm/memory.c:5144 [inline]
>  __handle_mm_fault+0x23b7/0x72b0 mm/memory.c:5285
>  handle_mm_fault+0x27e/0x770 mm/memory.c:5450
>  do_user_addr_fault arch/x86/mm/fault.c:1364 [inline]
>  handle_page_fault arch/x86/mm/fault.c:1507 [inline]
>  exc_page_fault+0x456/0x870 arch/x86/mm/fault.c:1563
>  asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:570
> 
> The root cause is: in f2fs_filemap_fault(), vmf->vma may be not alive after
> filemap_fault(), so it may cause use-after-free issue when accessing
> vmf->vma->vm_flags in trace_f2fs_filemap_fault(). So it needs to keep vm_flags
> in separated temporary variable for tracepoint use.
> 
> Fixes: 87f3afd366f7 ("f2fs: add tracepoint for f2fs_vm_page_mkwrite()")
> Reported-and-tested-by: syzbot+763afad57075d3f862f2@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://lore.kernel.org/lkml/000000000000e8222b060f00db3b@xxxxxxxxxx
> Cc: Ed Tsai <Ed.Tsai@xxxxxxxxxxxx>
> Cc: Hillf Danton <hdanton@xxxxxxxx>

Suggested-by: Hillf Danton <hdanton@xxxxxxxx>

I modified to "Suggested-by". Please let me know if this doesn't work.

> Signed-off-by: Chao Yu <chao@xxxxxxxxxx>
> ---
>  fs/f2fs/file.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index a47c57e813bb..c19e55a3e50e 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -39,6 +39,7 @@
>  static vm_fault_t f2fs_filemap_fault(struct vm_fault *vmf)
>  {
>  	struct inode *inode = file_inode(vmf->vma->vm_file);
> +	vm_flags_t flags = vmf->vma->vm_flags;
>  	vm_fault_t ret;
>  
>  	ret = filemap_fault(vmf);
> @@ -46,7 +47,7 @@ static vm_fault_t f2fs_filemap_fault(struct vm_fault *vmf)
>  		f2fs_update_iostat(F2FS_I_SB(inode), inode,
>  					APP_MAPPED_READ_IO, F2FS_BLKSIZE);
>  
> -	trace_f2fs_filemap_fault(inode, vmf->pgoff, vmf->vma->vm_flags, ret);
> +	trace_f2fs_filemap_fault(inode, vmf->pgoff, flags, ret);
>  
>  	return ret;
>  }
> -- 
> 2.40.1