ext4: fix use-after-free in ext4_orphan_cleanup

[PATCH] ext4: fix use-after-free in ext4_orphan_cleanup

Posted by Baokun Li 3 years, 5 months ago

I caught a issue as follows:
==================================================================
 BUG: KASAN: use-after-free in __list_add_valid+0x28/0x1a0
 Read of size 8 at addr ffff88814b13f378 by task mount/710

 CPU: 1 PID: 710 Comm: mount Not tainted 6.1.0-rc3-next #370
 Call Trace:
  <TASK>
  dump_stack_lvl+0x73/0x9f
  print_report+0x25d/0x759
  kasan_report+0xc0/0x120
  __asan_load8+0x99/0x140
  __list_add_valid+0x28/0x1a0
  ext4_orphan_cleanup+0x564/0x9d0 [ext4]
  __ext4_fill_super+0x48e2/0x5300 [ext4]
  ext4_fill_super+0x19f/0x3a0 [ext4]
  get_tree_bdev+0x27b/0x450
  ext4_get_tree+0x19/0x30 [ext4]
  vfs_get_tree+0x49/0x150
  path_mount+0xaae/0x1350
  do_mount+0xe2/0x110
  __x64_sys_mount+0xf0/0x190
  do_syscall_64+0x35/0x80
  entry_SYSCALL_64_after_hwframe+0x63/0xcd
  </TASK>
 [...]
==================================================================

Above issue may happen as follows:
-------------------------------------
ext4_fill_super
  ext4_orphan_cleanup
   --- loop1: assume last_orphan is 12 ---
    list_add(&EXT4_I(inode)->i_orphan, &EXT4_SB(sb)->s_orphan)
    ext4_truncate --> return 0
      ext4_inode_attach_jinode --> return -ENOMEM
    iput(inode) --> free inode<12>
   --- loop2: last_orphan is still 12 ---
    list_add(&EXT4_I(inode)->i_orphan, &EXT4_SB(sb)->s_orphan);
    // use inode<12> and trigger UAF

To solve this issue, we need to propagate the return value of
ext4_inode_attach_jinode() appropriately.

Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
 fs/ext4/inode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index fca47470c85a..e7e7561f0baa 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4224,7 +4224,8 @@ int ext4_truncate(struct inode *inode)
 
 	/* If we zero-out tail of the page, we have to create jinode for jbd2 */
 	if (inode->i_size & (inode->i_sb->s_blocksize - 1)) {
-		if (ext4_inode_attach_jinode(inode) < 0)
+		err = ext4_inode_attach_jinode(inode);
+		if (err)
 			goto out_trace;
 	}
 
-- 
2.31.1

Re: [PATCH] ext4: fix use-after-free in ext4_orphan_cleanup

Posted by Theodore Ts'o 3 years, 4 months ago

On Wed, 2 Nov 2022 16:06:33 +0800, Baokun Li wrote:
> I caught a issue as follows:
> ==================================================================
>  BUG: KASAN: use-after-free in __list_add_valid+0x28/0x1a0
>  Read of size 8 at addr ffff88814b13f378 by task mount/710
> 
>  CPU: 1 PID: 710 Comm: mount Not tainted 6.1.0-rc3-next #370
>  Call Trace:
>   <TASK>
>   dump_stack_lvl+0x73/0x9f
>   print_report+0x25d/0x759
>   kasan_report+0xc0/0x120
>   __asan_load8+0x99/0x140
>   __list_add_valid+0x28/0x1a0
>   ext4_orphan_cleanup+0x564/0x9d0 [ext4]
>   __ext4_fill_super+0x48e2/0x5300 [ext4]
>   ext4_fill_super+0x19f/0x3a0 [ext4]
>   get_tree_bdev+0x27b/0x450
>   ext4_get_tree+0x19/0x30 [ext4]
>   vfs_get_tree+0x49/0x150
>   path_mount+0xaae/0x1350
>   do_mount+0xe2/0x110
>   __x64_sys_mount+0xf0/0x190
>   do_syscall_64+0x35/0x80
>   entry_SYSCALL_64_after_hwframe+0x63/0xcd
>   </TASK>
>  [...]
> ==================================================================
> 
> [...]

Applied, thanks!

[1/1] ext4: fix use-after-free in ext4_orphan_cleanup
      commit: ad7ab3c3f740ce379e230d28c6aa8f9550182841

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>

Re: [PATCH] ext4: fix use-after-free in ext4_orphan_cleanup

Posted by Jan Kara 3 years, 5 months ago

On Wed 02-11-22 16:06:33, Baokun Li wrote:
> I caught a issue as follows:
> ==================================================================
>  BUG: KASAN: use-after-free in __list_add_valid+0x28/0x1a0
>  Read of size 8 at addr ffff88814b13f378 by task mount/710
> 
>  CPU: 1 PID: 710 Comm: mount Not tainted 6.1.0-rc3-next #370
>  Call Trace:
>   <TASK>
>   dump_stack_lvl+0x73/0x9f
>   print_report+0x25d/0x759
>   kasan_report+0xc0/0x120
>   __asan_load8+0x99/0x140
>   __list_add_valid+0x28/0x1a0
>   ext4_orphan_cleanup+0x564/0x9d0 [ext4]
>   __ext4_fill_super+0x48e2/0x5300 [ext4]
>   ext4_fill_super+0x19f/0x3a0 [ext4]
>   get_tree_bdev+0x27b/0x450
>   ext4_get_tree+0x19/0x30 [ext4]
>   vfs_get_tree+0x49/0x150
>   path_mount+0xaae/0x1350
>   do_mount+0xe2/0x110
>   __x64_sys_mount+0xf0/0x190
>   do_syscall_64+0x35/0x80
>   entry_SYSCALL_64_after_hwframe+0x63/0xcd
>   </TASK>
>  [...]
> ==================================================================
> 
> Above issue may happen as follows:
> -------------------------------------
> ext4_fill_super
>   ext4_orphan_cleanup
>    --- loop1: assume last_orphan is 12 ---
>     list_add(&EXT4_I(inode)->i_orphan, &EXT4_SB(sb)->s_orphan)
>     ext4_truncate --> return 0
>       ext4_inode_attach_jinode --> return -ENOMEM
>     iput(inode) --> free inode<12>
>    --- loop2: last_orphan is still 12 ---
>     list_add(&EXT4_I(inode)->i_orphan, &EXT4_SB(sb)->s_orphan);
>     // use inode<12> and trigger UAF
> 
> To solve this issue, we need to propagate the return value of
> ext4_inode_attach_jinode() appropriately.
> 
> Signed-off-by: Baokun Li <libaokun1@huawei.com>

Nice catch. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>
	
								Honza

> ---
>  fs/ext4/inode.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index fca47470c85a..e7e7561f0baa 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4224,7 +4224,8 @@ int ext4_truncate(struct inode *inode)
>  
>  	/* If we zero-out tail of the page, we have to create jinode for jbd2 */
>  	if (inode->i_size & (inode->i_sb->s_blocksize - 1)) {
> -		if (ext4_inode_attach_jinode(inode) < 0)
> +		err = ext4_inode_attach_jinode(inode);
> +		if (err)
>  			goto out_trace;
>  	}
>  
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR