linux-ext4 - Re: [PATCH v2] tune2fs: allow disabling casefold feature

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]

Date:   Thu, 7 Jul 2022 19:50:53 +0000
From:   Eric Biggers <ebiggers@...nel.org>
To:     Slava Bacherikov <slava@...her09.org>
Cc:     tytso@....edu, linux-ext4@...r.kernel.org, krisman@...labora.com
Subject: Re: [PATCH v2] tune2fs: allow disabling casefold feature

On Thu, Jul 07, 2022 at 10:04:56PM +0300, Slava Bacherikov wrote:
> diff --git a/misc/tune2fs.8.in b/misc/tune2fs.8.in
> index 628dcdc0..8ef28860 100644
> --- a/misc/tune2fs.8.in
> +++ b/misc/tune2fs.8.in
> @@ -593,8 +593,10 @@ Enable the file system to be larger than 2^32 blocks.
>  .TP
>  .B casefold
>  Enable support for file system level casefolding.
> -.B Tune2fs
> -currently only supports setting this file system feature.
> +The option could be disabled only if filesystem has no
> +directories with
> +.B F
> +attribute.

Please use present tense: "could" => "can".  Otherwise this can be interpreted
as the opposite of what you meant.

Also, "cleared" instead of "disabled", to be consistent with the rest of the
page.

>  static errcode_t disable_uninit_bg(ext2_filsys fs, __u32 csum_feature_flag)
>  {
>  	struct ext2_group_desc *gd;
> @@ -1554,6 +1590,20 @@ mmp_error:
>  		enabling_casefold = 1;
>  	}
>  
> +	if (FEATURE_OFF(E2P_FEATURE_INCOMPAT, EXT4_FEATURE_INCOMPAT_CASEFOLD)) {
> +		if (mount_flags & EXT2_MF_MOUNTED) {
> +			fputs(_("The casefold feature may only be disabled when "
> +				"the filesystem is unmounted.\n"), stderr);
> +			return 1;
> +		}
> +		if (has_casefold_inode(fs)) {
> +			fputs(_("The casefold feature couldn't be disabled when "
> +					"there are inodes with +F flag.\n"), stderr);
> +			return 1;
> +		}
> +		enabling_casefold = 0;

Likewise, "couldn't" => "can't".

Also, what are the semantics of disabling casefold, exactly?  Do the encoding
and encoding flags fields in the superblock also get cleared?

- Eric