linux-ext4 - RE: [PATCH 1/2] ext4, project: expand inode extra size if possible

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]
Date:   Fri, 30 Jun 2017 09:54:49 +0000
From:   Wang Shilong <wshilong@....com>
To:     "miaoxie@...wei.com" <miaoxie@...wei.com>,
        Wang Shilong <wangshilong1991@...il.com>,
        "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>
CC:     "tytso@....edu" <tytso@....edu>, Li Xi <lixi@....com>,
        "yi.zhang@...wei.com" <yi.zhang@...wei.com>,
        "adilger@...ger.ca" <adilger@...ger.ca>,
        Shuichi Ihara <sihara@....com>
Subject: RE: [PATCH 1/2] ext4, project: expand inode extra size if possible


________________________________________
From: Miao Xie [miaoxie@...wei.com]
Sent: Friday, June 30, 2017 16:48
To: Wang Shilong; linux-ext4@...r.kernel.org
Cc: tytso@....edu; Li Xi; yi.zhang@...wei.com; adilger@...ger.ca; Wang Shilong; Shuichi Ihara
Subject: Re: [PATCH 1/2] ext4, project: expand inode extra size if possible

on 2017/6/30 at 11:51, Wang Shilong wrote:
> when upgrading from old format, try to set project id
> to old file first time, it will return EOVERFLOW, but if
> that file is dirtied(touch etc), changing project id will
> be allowed, this might be confusing for users, we could
> try to expand @i_extra_iszie here too.
>
> Reported-by: zhangyi(F) <yi.zhang@...wei.com>
> Signed-off-by: Wang Shilong <wshilong@....com>
> ---
>  fs/ext4/ext4.h  |  3 +++
>  fs/ext4/inode.c |  7 +++----
>  fs/ext4/ioctl.c | 17 +++++++++++++++--
>  3 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 3219154..640f006 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2453,6 +2453,9 @@ int ext4_walk_page_buffers(handle_t *handle,
>                          int *partial,
>                          int (*fn)(handle_t *handle,
>                                    struct buffer_head *bh));
> +int ext4_expand_extra_isize(struct inode *inode,
> +                         unsigned int new_extra_isize,
> +                         struct ext4_iloc iloc, handle_t *handle);
>  int do_journal_get_write_access(handle_t *handle,
>                               struct buffer_head *bh);
>  #define FALL_BACK_TO_NONDELALLOC 1
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 5cf82d0..d53fae6 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5632,10 +5632,9 @@ ext4_reserve_inode_write(handle_t *handle, struct inode *inode,
>   * Expand an inode by new_extra_isize bytes.
>   * Returns 0 on success or negative error number on failure.
>   */
> -static int ext4_expand_extra_isize(struct inode *inode,
> -                                unsigned int new_extra_isize,
> -                                struct ext4_iloc iloc,
> -                                handle_t *handle)
> +int ext4_expand_extra_isize(struct inode *inode,
> +                         unsigned int new_extra_isize,
> +                         struct ext4_iloc iloc, handle_t *handle)
>  {
>       struct ext4_inode *raw_inode;
>       struct ext4_xattr_ibody_header *header;
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index 0c21e22..819f59d 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -319,6 +319,7 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
>       struct ext4_iloc iloc;
>       struct ext4_inode *raw_inode;
>       struct dquot *transfer_to[MAXQUOTAS] = { };
> +     bool need_expand = false;
>
>       if (!ext4_has_feature_project(sb)) {
>               if (projid != EXT4_DEF_PROJID)
> @@ -350,7 +351,10 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
>               goto out_unlock;
>
>       raw_inode = ext4_raw_inode(&iloc);
> -     if (!EXT4_FITS_IN_INODE(raw_inode, ei, i_projid)) {
> +     if (!EXT4_FITS_IN_INODE(raw_inode, ei, i_projid) &&
> +         !ext4_test_inode_state(inode, EXT4_STATE_NO_EXPAND)) {
> +             need_expand = true;
> +     } else if (!EXT4_FITS_IN_INODE(raw_inode, ei, i_projid)) {
>               err = -EOVERFLOW;
>               brelse(iloc.bh);
>               goto out_unlock;
> @@ -361,12 +365,21 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
>
>       handle = ext4_journal_start(inode, EXT4_HT_QUOTA,
>               EXT4_QUOTA_INIT_BLOCKS(sb) +
> -             EXT4_QUOTA_DEL_BLOCKS(sb) + 3);
> +             EXT4_QUOTA_DEL_BLOCKS(sb) + 3 +
> +             need_expand ? EXT4_DATA_TRANS_BLOCKS(sb) : 0);
>       if (IS_ERR(handle)) {
>               err = PTR_ERR(handle);
>               goto out_unlock;
>       }
>
> +     if (need_expand) {
> +             err = ext4_expand_extra_isize(inode,
> +                                   EXT4_SB(sb)->s_want_extra_isize,
> +                                   iloc, handle);
> +             if (err)
> +                     goto out_stop;
> +     }
> +

ext4_expand_extra_isize should be invoked after ext4_reserve_inode_write.

--->Yup, indeed.

And I think it is better to restructure ext4_expand_extra_isize by moving NO_EXPAND check,
nojournal check and journal credits extend into it, and then we just if i_projid is in
the inode or not, if not, invoke ext4_expand_extra_isize. (don't forget to do cleanup for
ext4_mark_inode_dirty), And then we can remove many check in the above code.

--->thanks for good suggestions, will fix it.


Thanks
Miao

>       err = ext4_reserve_inode_write(handle, inode, &iloc);
>       if (err)
>               goto out_stop;
>