linux-ext4 - Re: [PATCH 05/12] ext4: let ext4_group_add

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: <CAOQ4uxjq-10_yZFhP1A=N6tTbdotVsyLm5=_GSK+UwpyS0GL0w@mail.gmail.com>
Date:	Mon, 18 Jul 2011 09:48:52 +0300
From:	Amir Goldstein <amir73il@...il.com>
To:	Yongqiang Yang <xiaoqiangnk@...il.com>
Cc:	linux-ext4@...r.kernel.org, aedilger@...il.com
Subject: Re: [PATCH 05/12] ext4: let ext4_group_add_blocks return an error code

On Mon, Jul 18, 2011 at 5:52 AM, Yongqiang Yang <xiaoqiangnk@...il.com> wrote:
> This patch lets ext4_group_add_blocks() return an error code if it fails,
> so that upper functions can handle error correctly.
>
> Signed-off-by: Yongqiang Yang <xiaoqiangnk@...il.com>
> ---
>  fs/ext4/ext4.h    |    2 +-
>  fs/ext4/mballoc.c |   19 +++++++++++++------
>  fs/ext4/resize.c  |   10 +++++++---
>  3 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index bbe81db..da7ab48 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1799,7 +1799,7 @@ extern void ext4_free_blocks(handle_t *handle, struct inode *inode,
>                             unsigned long count, int flags);
>  extern int ext4_mb_add_groupinfo(struct super_block *sb,
>                ext4_group_t i, struct ext4_group_desc *desc);
> -extern void ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
> +extern int ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
>                                ext4_fsblk_t block, unsigned long count);
>  extern int ext4_trim_fs(struct super_block *, struct fstrim_range *);
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index ea80c0b..33c41e6 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4664,7 +4664,7 @@ error_return:
>  *
>  * This marks the blocks as free in the bitmap and buddy.
>  */
> -void ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
> +int ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
>                         ext4_fsblk_t block, unsigned long count)
>  {
>        struct buffer_head *bitmap_bh = NULL;
> @@ -4685,15 +4685,21 @@ void ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
>         * Check to see if we are freeing blocks across a group
>         * boundary.
>         */
> -       if (bit + count > EXT4_BLOCKS_PER_GROUP(sb))
> -               goto error_return;
> +       if (bit + count > EXT4_BLOCKS_PER_GROUP(sb)) {
> +               ext4_warning(sb, "too much blocks added to group %u\n",
> +                            block_group);
> +               return -EINVAL;
> +       }
>
>        bitmap_bh = ext4_read_block_bitmap(sb, block_group);
>        if (!bitmap_bh)
> -               goto error_return;
> +               return -EIO;

Any reason you are skipping the goto in the 2 error cases above and
missing the ext4_std_error()?

> +
>        desc = ext4_get_group_desc(sb, block_group, &gd_bh);
> -       if (!desc)
> +       if (!desc) {
> +               err = -EIO;
>                goto error_return;
> +       }
>
>        if (in_range(ext4_block_bitmap(sb, desc), block, count) ||
>            in_range(ext4_inode_bitmap(sb, desc), block, count) ||
> @@ -4703,6 +4709,7 @@ void ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
>                ext4_error(sb, "Adding blocks in system zones - "
>                           "Block = %llu, count = %lu",
>                           block, count);
> +               err = -EINVAL;
>                goto error_return;
>        }
>
> @@ -4771,7 +4778,7 @@ void ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
>  error_return:
>        brelse(bitmap_bh);
>        ext4_std_error(sb, err);
> -       return;
> +       return err;
>  }
>
>  /**
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index 374fd1c..2e63376 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -990,7 +990,7 @@ int ext4_group_extend(struct super_block *sb, struct ext4_super_block *es,
>        ext4_grpblk_t add;
>        struct buffer_head *bh;
>        handle_t *handle;
> -       int err;
> +       int err, err2;
>        ext4_group_t group;
>
>        o_blocks_count = ext4_blocks_count(es);
> @@ -1066,11 +1066,15 @@ int ext4_group_extend(struct super_block *sb, struct ext4_super_block *es,
>        ext4_debug("freeing blocks %llu through %llu\n", o_blocks_count,
>                   o_blocks_count + add);
>        /* We add the blocks to the bitmap and set the group need init bit */
> -       ext4_group_add_blocks(handle, sb, o_blocks_count, add);
> +       err = ext4_group_add_blocks(handle, sb, o_blocks_count, add);
>        ext4_handle_dirty_super(handle, sb);
>        ext4_debug("freed blocks %llu through %llu\n", o_blocks_count,
>                   o_blocks_count + add);
> -       if ((err = ext4_journal_stop(handle)))
> +       err2 = ext4_journal_stop(handle);
> +       if (!err && err2)
> +               err = err2;
> +
> +       if (err)
>                goto exit_put;
>
>        if (test_opt(sb, DEBUG))
> --
> 1.7.5.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html