linux-kernel - Re: [f2fs-dev] [PATCH] f2fs: remove circular locking between sb_internal and fs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACOAw_xOOcoTCYnxtSq9Sqn0aUvt=F_6UgUwWeh9RkS5Oj-RDg@mail.gmail.com>
Date:   Tue, 26 Oct 2021 10:56:36 -0700
From:   Daeho Jeong <daeho43@...il.com>
To:     Chao Yu <chao@...nel.org>
Cc:     linux-kernel@...r.kernel.org,
        linux-f2fs-devel@...ts.sourceforge.net, kernel-team@...roid.com,
        Daeho Jeong <daehojeong@...gle.com>
Subject: Re: [f2fs-dev] [PATCH] f2fs: remove circular locking between
 sb_internal and fs_reclaim

> Yes, I doubt the problem may also happen in f2fs_evict_inode() with below
> callpath:
>
> - evict_inode
>   - dquot_initialize failed
>   - sb_start_intwrite
>   - f2fs_truncate
>    - dquot_initialize lock dqio_sem
>
> How about this?
>

Got it~
Then we need this in both f2fs_evict_inode() and f2fs_drop_inode().

Thanks,

> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> old mode 100644
> new mode 100755
> index b24b9bc..0e49593
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -728,6 +728,7 @@ enum {
>       FI_ENABLE_COMPRESS,    /* enable compression in "user" compression mode */
>       FI_COMPRESS_RELEASED,    /* compressed blocks were released */
>       FI_ALIGNED_WRITE,    /* enable aligned write */
> +    FI_QUOTA_INIT_FAIL,    /* inidicate failed to initialize quota in drop_inode()/evict_inode() */
>       FI_MAX,            /* max flag, never be used */
>   };
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> old mode 100644
> new mode 100755
> index 13deae0..2fb53f54
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -789,9 +789,11 @@ int f2fs_truncate(struct inode *inode)
>           return -EIO;
>       }
>
> -    err = dquot_initialize(inode);
> -    if (err)
> -        return err;
> +    if (!is_inode_flag_set(inode, FI_QUOTA_INIT_FAIL)) {
> +        err = dquot_initialize(inode);
> +        if (err)
> +            return err;
> +    }
>
>       /* we should check inline_data size */
>       if (!f2fs_may_inline_data(inode)) {
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> old mode 100644
> new mode 100755
> index 1213f15..16cf50c
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -758,6 +758,7 @@ void f2fs_evict_inode(struct inode *inode)
>       if (err) {
>           err = 0;
>           set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR);
> +        set_inode_flag(inode, FI_QUOTA_INIT_FAIL);
>       }
>
>       f2fs_remove_ino_entry(sbi, inode->i_ino, APPEND_INO);
> @@ -770,6 +771,8 @@ void f2fs_evict_inode(struct inode *inode)
>   retry:
>       if (F2FS_HAS_BLOCKS(inode))
>           err = f2fs_truncate(inode);
> +    if (is_inode_flag_set(inode, FI_QUOTA_INIT_FAIL))
> +        clear_inode_flag(inode, FI_QUOTA_INIT_FAIL);
>
>       if (time_to_inject(sbi, FAULT_EVICT_INODE)) {
>           f2fs_show_injection_info(sbi, FAULT_EVICT_INODE);
>
> Thanks,
>
>
> > In fact, I picked up the idea from here.
> >
> >          err = dquot_initialize(inode);
> >          if (err) {
> >                  err = 0;
> >                  set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR);
> >          }
> >
> >          f2fs_remove_ino_entry(sbi, inode->i_ino, APPEND_INO);
> >          f2fs_remove_ino_entry(sbi, inode->i_ino, UPDATE_INO);
> >          f2fs_remove_ino_entry(sbi, inode->i_ino, FLUSH_INO);
> >
> >          sb_start_intwrite(inode->i_sb);
> >          set_inode_flag(inode, FI_NO_ALLOC);
> >          i_size_write(inode, 0);
> > retry:
> >          if (F2FS_HAS_BLOCKS(inode))
> >                  err = f2fs_truncate(inode);
> >