On Mon, 2021-04-19 at 12:54 -0700, Eric Biggers wrote: > 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@xxxxxxxxxx> 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. Ok, thanks. I'll plan to drop the invalidation from the ioctl codepaths, and leave it up to fscrypt_d_revalidate to sort out. -- Jeff Layton <jlayton@xxxxxxxxxx>
