linux-ext4 - Re: [RFC][PATCH 8/11][take 2] 48-bit extents in e2fsprogs

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: <20070621211732.GB5181@schatzie.adilger.int>
Date:	Thu, 21 Jun 2007 15:17:32 -0600
From:	Andreas Dilger <adilger@...sterfs.com>
To:	Valerie Clement <valerie.clement@...l.net>
Cc:	Theodore Tso <tytso@....edu>,
	ext4 development <linux-ext4@...r.kernel.org>
Subject: Re: [RFC][PATCH 8/11][take 2] 48-bit extents in e2fsprogs

On Jun 21, 2007  17:31 +0200, Valerie Clement wrote:
> +#define EXT4_EE_START(s, e)						\
> +	((e)->ee_start +						\
> +	(((s)->s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT) &&	\
> +	 (EXT2_BLOCKS_COUNT(s) > ((unsigned) 1 << 31)) ?		\
> +	 (__u64)(e)->ee_start_hi << 32 : 0))

Isn't the INCOMPAT_64BIT check already part of EXT2_BLOCKS_COUNT()?
It should be impossible to have the "> (1 << 31)" check be true for
a non-64-bit filesystem in that case.  Also, why check for > (1 << 31)
instead of >= (1 << 32)?

> +#define EXT4_EE_START_SET(s,e,blk)					\
> +	do {								\
> +		(e)->ee_start = (blk);					\
> +		if ((s)->s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT \
> +		    && EXT2_BLOCKS_COUNT(s) > ((unsigned) 1 << 31))	\
> +		(e)->ee_start_hi = (__u64)(blk) >> 32;			\

Here you should always set ee_start_hi = 0, so there is no need for
a conditional here.  It is only needed for the read because initial versions
of the extents code didn't clear ee_start_hi, so it deserves making a comment
to that effect above EXT4_EE_START() and EXT4_EI_LEAF().  One day we may
want to remove that hack and assume *_hi == 0 for all filesystems.

> +#define EXT4_EI_LEAF_SET(s,e,blk)					\
> +	do {								\
> +		(ix)->ei_leaf = (blk);					\
> +		if ((s)->s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT \
> +		    && EXT2_BLOCKS_COUNT(s) > ((unsigned) 1 << 31))	\
> +			(ix)->ei_leaf_hi = (__u64)(blk) >> 32;		\
> +	} while(0)

Same as above.

> @@ -1604,6 +1604,10 @@ static int e2fsck_ext_block_verify(struc
>  	e2fsck_t ctx = p->ctx;
>  	struct problem_context *pctx = p->pctx;
>  	int i, problem = 0;
> +	int flag_64bit;
> +
> +	flag_64bit = p->ctx->fs->super->s_feature_incompat &
> +			EXT4_FEATURE_INCOMPAT_64BIT;

Presumably we have verified INCOMPAT_64BIT is set for filesystems with
more than 2^32 blocks in the superblock and the device size?


Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

-
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