Re: [RFC PATCH v6 20/20] ceph: add fscrypt ioctls

[Luis Henriques <lhenriques@suse.de>]

Eric Biggers <ebiggers@kernel.org> writes:

> On Mon, Apr 19, 2021 at 08:19:59AM -0400, Jeff Layton wrote:
>> On Mon, 2021-04-19 at 11:09 +0100, Luis Henriques wrote:
>> > Hi Jeff!
>> > 
>> > Jeff Layton <jlayton@kernel.org> writes:
>> > <...>
>> > > +
>> > > +	case FS_IOC_ADD_ENCRYPTION_KEY:
>> > > +		ret = vet_mds_for_fscrypt(file);
>> > > +		if (ret)
>> > > +			return ret;
>> > > +		atomic_inc(&ci->i_shared_gen);
>> > 
>> > After spending some (well... a lot, actually) time looking at the MDS code
>> > to try to figure out my bug, I'm back at this point in the kernel client
>> > code.  I understand that this code is trying to invalidate the directory
>> > dentries here.  However, I just found that the directory we get at this
>> > point is the filesystem root directory, and not the directory we're trying
>> > to unlock.
>> > 
>> > So, I still don't fully understand the issue I'm seeing, but I believe the
>> > code above is assuming 'ci' is the inode being unlocked, which isn't
>> > correct.
>> > 
>> > (Note: I haven't checked if there are other ioctls getting the FS root.)
>> > 
>> > Cheers,
>> 
>> 
>> Oh, interesting. That was my assumption. I'll have to take a look more
>> closely at what effect that might have then.
>> 
>
> FS_IOC_ADD_ENCRYPTION_KEY, FS_IOC_REMOVE_ENCRYPTION_KEY,
> FS_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS, and FS_IOC_GET_ENCRYPTION_KEY_STATUS can
> all be executed on any file or directory on the filesystem (but preferably on
> the root directory) because they are operations on the filesystem, not on any
> specific file or directory.  They deal with encryption keys, which can protect
> any number of encrypted directories (even 0 or a large number) and/or even loose
> encrypted files that got moved into an unencrypted directory.
>
> Note that this is all described in the documentation
> (https://www.kernel.org/doc/html/latest/filesystems/fscrypt.html).
> If the documentation is unclear please suggest improvements to it.
>
> Also, there shouldn't be any need for FS_IOC_ADD_ENCRYPTION_KEY to invalidate
> dentries itself because that is the point of fscrypt_d_revalidate(); the
> invalidation happens on-demand later.

I think the documentation is very clear regarding these ioctls.  I guess I
just need to go refresh my memory as I have read that document long time
ago.  Thanks for reminding me to do that ;-)

Cheers,
-- 
Luis