Re: [PATCH] e2fsck: set the encrypt feature if the filesystem has encrypted inodes

[Eric Biggers <ebiggers@kernel.org>]

On Fri, Nov 13, 2026 at 05:21:41PM +0100, Alberto Garcia wrote:
> A filesystem without the encrypt feature can still contain encrypted
> inodes, either due to corruption or because the image was crafted that
> way.
> 
> e2fsck currently accepts such inodes without complaint. Since those
> inodes can be decrypted, this should be fixed by setting the encrypt
> feature on the filesystem.
> 
> Note that the Linux kernel does not currently support encryption when
> the block size is larger than the page size. After this change, such
> a filesystem will be rejected at mount time (see the kernel commit
> 709f0f1f1bf5, "ext4: add checks for large folio incompatibilities
> when BS > PS"). However, since this depends on the page size of the
> machine where the filesystem is mounted, and the kernel could lift
> that limitation in the future, e2fsck does not take it into account.
> 
> Fix this by adding a new problem (PR_1_ENCRYPT_FEATURE) and a
> regression test.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>

It's probably safe to do this by now, as I mentioned previously
(https://lore.kernel.org/linux-ext4/20260925190751.GB2060@quark/).

A few comments on the diff:

>  tests/f_encrypt_feature_missing/image.gz   | Bin 0 -> 5566 bytes

This image has a 4096-byte block of incompressible ciphertext in it.  An
empty file would exercise this new code just as well and result in a
filesystem image of around 1.5 KB.

> @@ -389,6 +389,13 @@ int add_encrypted_file(e2fsck_t ctx, struct problem_context *pctx)
>  		return 0;

The function comment of add_encrypted_file() should be updated to
mention the new behavior.

> +	/* The inode is encrypted, so the filesystem needs the encrypt feature */
> +	if (!ext2fs_has_feature_encrypt(ctx->fs->super) &&
> +	    fix_problem(ctx, PR_1_ENCRYPT_FEATURE, pctx)) {
> +		ext2fs_set_feature_encrypt(ctx->fs->super);
> +		ext2fs_mark_super_dirty(ctx->fs);
> +	}

I'm a little surprised the fix is to set the feature flag, rather than
clear the inode.  It looks like e2fsck does this for other feature flags
though, so I guess this is consistent with that.

Given that it's done this way, though, let's consider the case where
the user declines to fix this problem, either interactively or via
'e2fsck -n'.  In that case, the problem gets reported for all
thousands/millions of inodes that could have the problem.  Perhaps it
should be reported only for the first, by checking
'info->file_ranges_count == 0'?

> diff --git a/e2fsck/problem.c b/e2fsck/problem.c
> index e433281f..d8766a65 100644
> --- a/e2fsck/problem.c
> +++ b/e2fsck/problem.c
> @@ -1319,6 +1319,14 @@ static struct e2fsck_problem problem_table[] = {
>  	  N_("@i %i references EA inode but @S is missing EA_INODE feature\n"),
>  	  PROMPT_FIX, PR_PREEN_OK, 0, 0, 0 },
>  
> +	/*
> +	 * Encrypted inode found but the filesystem is missing the
> +	 * encrypt feature
> +	 */
> +	{ PR_1_ENCRYPT_FEATURE,
> +	  N_("@i %i is encrypted but @S is missing ENCRYPT feature\n"),
> +	  PROMPT_FIX, PR_PREEN_OK, 0, 0, 0 },

Isn't this missing '/* xgettext:no-c-format */'?

- Eric