linux-ext4 - Re: [PATCH] ext4: check orig

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: <2BA05350-23D4-47B5-8D9C-031C45F0BC65@dilger.ca>
Date:   Tue, 19 Jun 2018 22:31:05 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     Chengguang Xu <cgxu519@....com>
Cc:     Theodore Ts'o <tytso@....edu>,
        Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] ext4: check orig_data in remount


> On Jun 6, 2018, at 2:26 AM, Chengguang Xu <cgxu519@....com> wrote:
> 
> Currently duplicate data to orig_data in ext4_remount()
> but there is lack of check for duplication result.
> This patch adds proper check for it.
> 
> Signed-off-by: Chengguang Xu <cgxu519@....com>

Thank you for the patch.  I think it looks correct, but I was
initially confused by the commit message.  I think something
like the following would be more clear:

    There is no check for allocation failure when duplicating
    "orig_data" in ext4_remount().  Check for failure and
    return an error in this case.


While I don't think it is (currently) possible for a one-page
allocation to fail (always true because of copy_mount_options()
used in ksys_mount()), I prefer to always check for allocation
failures in case this ever changes in the future, or if there
is allocation fault injection or other fuzzing turned on.

In any case, you can add:

Reviewed-by: Andreas Dilger <adilger@...ger.ca>

> ---
> fs/ext4/super.c | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 00fe75a71c4b..cba245460a9c 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -5014,6 +5014,9 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
> #endif
> 	char *orig_data = kstrdup(data, GFP_KERNEL);
> 
> +	if (data && !orig_data)
> +		return -ENOMEM;
> +
> 	/* Store the original options */
> 	old_sb_flags = sb->s_flags;
> 	old_opts.s_mount_opt = sbi->s_mount_opt;
> --
> 2.17.1
> 


Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)