linux-ext4 - Re: [PATCH v4 6/7] ext2fs: add EXT4_FEATURE_INCOMPAT

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: <427DB250-4A54-4DA2-8A38-F488F95313F3@dilger.ca>
Date:   Fri, 4 May 2018 03:37:21 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     Artem Blagodarenko <artem.blagodarenko@...il.com>
Cc:     Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH v4 6/7] ext2fs: add EXT4_FEATURE_INCOMPAT_64INODE support

On May 4, 2018, at 1:09 AM, c17828 <artem.blagodarenko@...il.com> wrote:
> 
> From: Artem Blagodarenko <artem.blagodarenko@...il.com>
> 
> Inodes count and free inodes count should be 64 bit long.
> This patch also changes s_inodes_count* to 64 bit
> 
> Lustre-bug: https://jira.hpdd.intel.com/browse/LU-9309
> Signed-off-by: Artem Blagodarenko <artem.blagodarenko@...il.com>
> 
> extern void e2fsck_add_dx_dir(e2fsck_t ctx, ext2_ino_t ino, int num_blocks);
> diff --git a/e2fsck/pass2.c b/e2fsck/pass2.c
> index a8ccc465..ef0b97c0 100644
> --- a/e2fsck/pass2.c
> +++ b/e2fsck/pass2.c
> @@ -179,7 +179,12 @@ void e2fsck_pass2(e2fsck_t ctx)
> 
> 	if (ext2fs_has_feature_dir_index(fs->super))
> 		ext2fs_dblist_sort2(fs->dblist, special_dir_block_cmp);
> -
> +	if (ext2fs_has_feature_inode64(ctx->fs->super) &&
> +	    !ext2fs_has_feature_dirdata(ctx->fs->super)) {
> +		if (fix_problem(ctx, PR_2_FIX_DIRDATA_FEATURE, &cd.pctx))
> +			ctx->fs->super->s_feature_incompat |=
> +				EXT4_FEATURE_INCOMPAT_DIRDATA;

This should use:
			 ext2fs_set_feature_dirdata(ctx->fs->super);

> diff --git a/e2fsck/problem.c b/e2fsck/problem.c
> index 2a86d528..d8620b93 100644
> --- a/e2fsck/problem.c
> +++ b/e2fsck/problem.c
> @@ -1676,6 +1676,11 @@ static struct e2fsck_problem problem_table[] = {
> 	  N_("@E dirdata length set incorrectly.\n"),
> 	  PROMPT_CLEAR, PR_PREEN_OK },
> 
> +	/* dirdata feature is needed for inode64 */
> +	{ PR_2_FIX_DIRDATA_FEATURE,
> +	  N_("@E ino64 feature without dirdata.\n"),

This should use "inode64" since that is what the feature is named in
lib/e2p/feature.c.

> diff --git a/lib/e2p/feature.c b/lib/e2p/feature.c
> index b7f6c1d2..fd77dedd 100644
> --- a/lib/e2p/feature.c
> +++ b/lib/e2p/feature.c
> @@ -105,6 +105,8 @@ static struct feature feature_list[] = {
> 			"inline_data"},
> 	{       E2P_FEATURE_INCOMPAT, EXT4_FEATURE_INCOMPAT_ENCRYPT,
> 			"encrypt"},
> +	{	E2P_FEATURE_INCOMPAT, EXT4_FEATURE_INCOMPAT_INODE64,
> +			"inode64"},
> 	{	0, 0, 0 },
> };
> 

> diff --git a/misc/mke2fs.c b/misc/mke2fs.c
> index 8edb7546..2878aadc 100644
> --- a/misc/mke2fs.c
> +++ b/misc/mke2fs.c
> 
> @@ -2457,13 +2458,15 @@ profile_error:
> 	if (num_inodes == 0) {
> 		unsigned long long n;
> 		n = ext2fs_blocks_count(&fs_param) * blocksize / inode_ratio;
> -		if (n > MAX_32_NUM) {
> -			if (ext2fs_has_feature_64bit(&fs_param))
> +		if (n > MAX_32_NUM && !ext2fs_has_feature_inode64(&fs_param)) {
> +			if (ext2fs_has_feature_64bit(&fs_param)) {
> 				num_inodes = MAX_32_NUM;
> +			}
> 			else {

(style) prefer "} else {"

Strangely, I don't see any changes in this patch that would affect the
ABI compatibility?  Looking at lib/ext2fs/ext2fs_*() interfaces that
take an ext2fs_ino_t there are quite a few places that need to be fixed.
Basically, any piece of code that is using ext2fs_get_inodes_count()
and friends to handle an inode number or "ext2fs_ino_t" needs to be
examined.

That will likely need at least one or two more patches before this feature
could be enabled and considered working.  One of the thoughts that Ted had
was to have a test mode for e2fsprogs (and also the kernel) where the high
word of the inode number is set to some constant on initial access for
filesystems with < 2^32 inodes, and then any time it is used the high word
is checked to ensure the high word has not been dropped.  That would ensure
this code is actually working with 64-bit inode numbers, since I suspect
that it isn't really working at this time.

Cheers, Andreas






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