linux-kernel - Re: [PATCH v2 6/8] tmpfs: Add flag FS_CASEFOLD

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: <500ff1ba-d8db-4f4d-9084-6d59401992da@igalia.com>
Date: Wed, 4 Sep 2024 19:28:16 -0300
From: André Almeida <andrealmeid@...lia.com>
To: Gabriel Krisman Bertazi <krisman@...e.de>
Cc: Hugh Dickins <hughd@...gle.com>, Andrew Morton
 <akpm@...ux-foundation.org>, Alexander Viro <viro@...iv.linux.org.uk>,
 Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
 krisman@...nel.org, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
 linux-fsdevel@...r.kernel.org, kernel-dev@...lia.com,
 Daniel Rosenberg <drosen@...gle.com>, smcv@...labora.com,
 Christoph Hellwig <hch@....de>
Subject: Re: [PATCH v2 6/8] tmpfs: Add flag FS_CASEFOLD_FL support for tmpfs
 dirs

Hi Krisman,

Thanks for the feedback!

Em 03/09/2024 13:15, Gabriel Krisman Bertazi escreveu:
> André Almeida <andrealmeid@...lia.com> writes:
> 
>> Enable setting flag FS_CASEFOLD_FL for tmpfs directories, when tmpfs is
>> mounted with casefold support. A special check is need for this flag,
>> since it can't be set for non-empty directories.
>>
>> Signed-off-by: André Almeida <andrealmeid@...lia.com>

[...]

>> +
>> +	if (fsflags & FS_CASEFOLD_FL) {
>> +		if (!sb->s_encoding)
>> +			return -EOPNOTSUPP;
>> +
>> +		if (!S_ISDIR(inode->i_mode))
>> +			return -ENOTDIR;
>> +
>> +		if (dentry && !simple_empty(dentry))
>> +			return -ENOTEMPTY;
>> +
>> +		i_flags |= S_CASEFOLD;
>> +	} else if (old & S_CASEFOLD) {
>> +		if (dentry && !simple_empty(dentry))
>> +			return -ENOTEMPTY;
> 
> We don't want to fail if a directory already has the S_CASEFOLD
> flag and we are not flipping it in the current operation.  Something like:
> 
> if ((fsflags ^ old) & S_CASEFOLD) {
> 	if (!sb->s_encoding)
> 		return -EOPNOTSUPP;
> 
> 	if (!S_ISDIR(inode->i_mode))
> 		return -ENOTDIR;
> 
> 	if (dentry && !simple_empty(dentry))
> 		return -ENOTEMPTY;
>          i_flags |= fsflags & S_CASEFOLD;
> }
> 

You are right, it's broken and failing for directories with S_CASEFOLD. 
Here's a small test showing that we can't add the +d attribute to a 
non-empty CI folder (+d doesn't require the directory to be empty):

folder ) mkdir A
folder ) mkdir A/B
folder ) chattr +d A/B
folder ) chattr +d A
chattr: Directory not empty while setting flags on A

However, FS_CASEFOLD_FL != S_CASEFOLD and the set of values for 
inode->i_flags (var old) and fsflags aren't the same, so your proposed 
snippet didn't work. I see that ext4 has a very similar code as your 
proposal, but I think they do something different with the flag values.

I rewrote my code separating the three possible paths and it worked:

/* inheritance from parent dir/keeping the same flags path */
if ((fsflags & FS_CASEFOLD_FL) && (old & S_CASEFOLD))
	i_flags |= S_CASEFOLD;

/* removing flag path */
if (!(fsflags & FS_CASEFOLD_FL) && (old & S_CASEFOLD))
	if (dentry && !simple_empty(dentry))
		return -ENOTEMPTY;

/* adding flag path */
if ((fsflags & FS_CASEFOLD_FL) && !(old & S_CASEFOLD)) {
	if (!sb->s_encoding)
		return -EOPNOTSUPP;

	if (!S_ISDIR(inode->i_mode))
		return -ENOTDIR;

	if (dentry && !simple_empty(dentry))
		return -ENOTEMPTY;

	i_flags |= S_CASEFOLD;
}

In that way, the `chattr +d` call doesn't fall into the simple_empty() 
check. I simplified the code like this for the v3:

if (fsflags & FS_CASEFOLD_FL) {
	if (!(old & S_CASEFOLD)) {
		if (!sb->s_encoding)
			return -EOPNOTSUPP;

		if (!S_ISDIR(inode->i_mode))
			return -ENOTDIR;

		if (dentry && !simple_empty(dentry))
			return -ENOTEMPTY;
	}

	i_flags |= S_CASEFOLD;
} else if (old & S_CASEFOLD) {
	if (dentry && !simple_empty(dentry))
		return -ENOTEMPTY;
}