linux-ext4 - Re: [PATCH] ext4: sparse fixes

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] [day] [month] [year] [list]

Message-ID: <20081103025911.GA29102@mit.edu>
Date:	Sun, 2 Nov 2008 21:59:12 -0500
From:	Theodore Tso <tytso@....edu>
To:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Cc:	cmm@...ibm.com, sandeen@...hat.com, frederic.bohe@...l.net,
	linux-ext4@...r.kernel.org
Subject: Re: [PATCH] ext4: sparse fixes

On Fri, Oct 31, 2008 at 10:25:35PM +0530, Aneesh Kumar K.V wrote:
> There are some locking fixes also in this patch.

Can we move the one real locking fix into a separate patch, and push
that to Linus ASAP, while leaving the cleanups to later?

> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index bdddea1..a725a5c 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1455,9 +1455,8 @@ static int ext4_fill_flex_info(struct super_block *sb)
>  
>  	/* We allocate both existing and potentially added groups */
>  	flex_group_count = ((sbi->s_groups_count + groups_per_flex - 1) +
> -			    ((sbi->s_es->s_reserved_gdt_blocks +1 ) <<
> -			      EXT4_DESC_PER_BLOCK_BITS(sb))) /
> -			   groups_per_flex;
> +			    ((le16_to_cpu(sbi->s_es->s_reserved_gdt_blocks) + 1) <<
> +			      EXT4_DESC_PER_BLOCK_BITS(sb))) / groups_per_flex;
>  	sbi->s_flex_groups = kzalloc(flex_group_count *
>  				     sizeof(struct flex_groups), GFP_KERNEL);
>  	if (sbi->s_flex_groups == NULL) {


This looks like a real bug fix which we should get to Linus quickly,
since could cause problems on Big Endian systems.

> @@ -2014,8 +2013,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
>  		       sb->s_id, le32_to_cpu(features));
>  		goto failed_mount;
>  	}
> -	has_huge_files = EXT4_HAS_RO_COMPAT_FEATURE(sb,
> -				    EXT4_FEATURE_RO_COMPAT_HUGE_FILE);
> +	has_huge_files = le32_to_cpu(EXT4_HAS_RO_COMPAT_FEATURE(sb,
> +				    EXT4_FEATURE_RO_COMPAT_HUGE_FILE));
>  	if (has_huge_files) {
>  		/*
>  		 * Large file size enabled file system can only be


This, on the other hand is a pointless sparse fixup that actually ends
up causing code bloat.  EXT4_HAS_RO_COMPAT() is a macro which
returns a boolean.  So we don't need to call le32_to_cpu() to it just
to shut up sparse.  Better to simply give a sparse anotation that this
returns an int and not a le32.

       		      	   	      	  - Ted
--
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