lkml.org 
[lkml]   [2023]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH 1/2] quota: fix null-ptr-deref in ext4_acquire_dquot()
    From
    Date


    On 2023/5/30 17:57, Jan Kara wrote:
    > On Sat 27-05-23 09:40:17, Ye Bin wrote:
    >> Syzbot found the following issue:
    >> Unable to handle kernel paging request at virtual address dfff800000000005
    >> KASAN: null-ptr-deref in range [0x0000000000000028-0x000000000000002f]
    > ...
    >> CPU: 0 PID: 6080 Comm: syz-executor747 Not tainted 6.3.0-rc7-syzkaller-g14f8db1c0f9a #0
    >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/14/2023
    >> pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
    >> pc : ext4_acquire_dquot+0x1d4/0x398 fs/ext4/super.c:6766
    >> lr : dquot_to_inode fs/ext4/super.c:6740 [inline]
    >> lr : ext4_acquire_dquot+0x1ac/0x398 fs/ext4/super.c:6766
    > OK, this is bad...
    >
    >> Above issue may happens as follows:
    >> ProcessA ProcessB ProcessC
    >> sys_fsconfig
    >> vfs_fsconfig_locked
    >> reconfigure_super
    >> ext4_remount
    >> dquot_suspend -> suspend all type quota
    >>
    >> sys_fsconfig
    >> vfs_fsconfig_locked
    >> reconfigure_super
    >> ext4_remount
    >> dquot_resume
    >> ret = dquot_load_quota_sb
    >> add_dquot_ref
    >> do_open -> open file O_RDWR
    >> vfs_open
    >> do_dentry_open
    >> get_write_access
    >> atomic_inc_unless_negative(&inode->i_writecount)
    >> ext4_file_open
    >> dquot_file_open
    >> dquot_initialize
    >> __dquot_initialize
    >> dqget
    >> if (!test_bit(DQ_ACTIVE_B, &dquot->dq_flags))
    >>
    >> __dquot_initialize
    >> __dquot_initialize
    >> dqget
    >> if (!test_bit(DQ_ACTIVE_B, &dquot->dq_flags))
    >> ext4_acquire_dquot -> Return error
    >> if (ret < 0)
    >> vfs_cleanup_quota_inode
    >> dqopt->files[type] = NULL;
    > But I don't see how this can happen. The code in dquot_load_quota_sb()
    > looks like:
    >
    > error = add_dquot_ref(sb, type);
    > if (error)
    > dquot_disable(sb, type, flags);
    >
    > So if an error happens in add_dquot_ref(), we'll call dquot_disable().
    > dquot_disable() then does:
    >
    > drop_dquot_ref(sb, cnt);
    > invalidate_dquots(sb, cnt);
    >
    > and invalidate_dquots() waits for reference count of all dquots to drop to
    > 0. Hence if dqget() returned a dquot pointer to ProcessC, then ProcessB
    > should wait until ProcessC drops the dquot reference (hence
    > ext4_acquire_dquot() is done).
    >
    > What am I missing?
    >
    > Honza
    My reproduction condition is:
    mkfs.ext4 -F /dev/sda
    tune2fs -Q usrquota /dev/sda

    dquot_disable
    ...
    if ((flags & DQUOT_USAGE_ENABLED && !(flags &
    DQUOT_LIMITS_ENABLED))
    || (flags & DQUOT_SUSPENDED && flags & (DQUOT_LIMITS_ENABLED |
    DQUOT_USAGE_ENABLED)))
    return -EINVAL;
    ...
    If without enable DQUOT_LIMITS_ENABLED dquot_disable() will just return
    -EINVAL.
    >> ext4_acquire_dquot
    >> -->dquot_to_inode(dquot) is NULL
    >>
    >> To solve above issue, if quota has been loaded, there's unneed to cleaup quota
    >> inode if dquot_load_quota_sb() return failed when do dquot_resume();
    >>
    >> Reported-by: syzbot+e633c79ceaecbf479854@syzkaller.appspotmail.com
    >> Signed-off-by: Ye Bin <yebin10@huawei.com>
    >> ---
    >> fs/quota/dquot.c | 2 +-
    >> 1 file changed, 1 insertion(+), 1 deletion(-)
    >>
    >> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
    >> index ffd40dc3e4e9..4b913faa48ec 100644
    >> --- a/fs/quota/dquot.c
    >> +++ b/fs/quota/dquot.c
    >> @@ -2476,7 +2476,7 @@ int dquot_resume(struct super_block *sb, int type)
    >> flags = dquot_generic_flag(flags, cnt);
    >> ret = dquot_load_quota_sb(sb, cnt, dqopt->info[cnt].dqi_fmt_id,
    >> flags);
    >> - if (ret < 0)
    >> + if (ret < 0 && !sb_has_quota_loaded(sb, cnt))
    >> vfs_cleanup_quota_inode(sb, cnt);
    >> }
    >>
    >> --
    >> 2.31.1
    >>

    \
     
     \ /
      Last update: 2023-06-03 08:15    [W:3.016 / U:1.520 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site