On Mon, Nov 23, 2020 at 02:51:44PM -0800, Eric Biggers wrote: > On Sun, Nov 22, 2020 at 01:12:18PM +0800, Gao Xiang wrote: > > Hi all, > > > > On Thu, Nov 19, 2020 at 06:09:03AM +0000, Daniel Rosenberg wrote: > > > This shifts the responsibility of setting up dentry operations from > > > fscrypt to the individual filesystems, allowing them to have their own > > > operations while still setting fscrypt's d_revalidate as appropriate. > > > > > > Most filesystems can just use generic_set_encrypted_ci_d_ops, unless > > > they have their own specific dentry operations as well. That operation > > > will set the minimal d_ops required under the circumstances. > > > > > > Since the fscrypt d_ops are set later on, we must set all d_ops there, > > > since we cannot adjust those later on. This should not result in any > > > change in behavior. > > > > > > Signed-off-by: Daniel Rosenberg <drosen@xxxxxxxxxx> > > > Acked-by: Eric Biggers <ebiggers@xxxxxxxxxx> > > > --- > > > > ... > > > > > extern const struct file_operations ext4_dir_operations; > > > > > > -#ifdef CONFIG_UNICODE > > > -extern const struct dentry_operations ext4_dentry_ops; > > > -#endif > > > - > > > /* file.c */ > > > extern const struct inode_operations ext4_file_inode_operations; > > > extern const struct file_operations ext4_file_operations; > > > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > > > index 33509266f5a0..12a417ff5648 100644 > > > --- a/fs/ext4/namei.c > > > +++ b/fs/ext4/namei.c > > > @@ -1614,6 +1614,7 @@ static struct buffer_head *ext4_lookup_entry(struct inode *dir, > > > struct buffer_head *bh; > > > > > > err = ext4_fname_prepare_lookup(dir, dentry, &fname); > > > + generic_set_encrypted_ci_d_ops(dentry); > > > > One thing might be worth noticing is that currently overlayfs might > > not work properly when dentry->d_sb->s_encoding is set even only some > > subdirs are CI-enabled but the others not, see generic_set_encrypted_ci_d_ops(), > > ovl_mount_dir_noesc => ovl_dentry_weird() > > > > For more details, see: > > https://android-review.googlesource.com/c/device/linaro/hikey/+/1483316/2#message-2e1f6ab0010a3e35e7d8effea73f60341f84ee4d > > > > Just found it by chance (and not sure if it's vital for now), and > > a kind reminder about this. > > > > Yes, overlayfs doesn't work on ext4 or f2fs filesystems that have the casefold > feature enabled, regardless of which directories are actually using casefolding. > This is an existing limitation which was previously discussed, e.g. at > https://lkml.kernel.org/linux-ext4/CAOQ4uxgPXBazE-g2v=T_vOvnr_f0ZHyKYZ4wvn7A3ePatZrhnQ@xxxxxxxxxxxxxx/T/#u > and > https://lkml.kernel.org/linux-ext4/20191203051049.44573-1-drosen@xxxxxxxxxx/T/#u. > > Gabriel and Daniel, is one of you still looking into fixing this? IIUC, the > current thinking is that when the casefolding flag is set on a directory, it's > too late to assign dentry_operations at that point. But what if all child > dentries (which must be negative) are invalidated first, and also the filesystem > forbids setting the casefold flag on encrypted directories that are accessed via > a no-key name (so that fscrypt_d_revalidate isn't needed -- i.e. the directory > would only go from "no d_ops" to "generic_ci_dentry_ops", not from > "generic_encrypted_dentry_ops" to "generic_encrypted_ci_dentry_ops")? >From my limited knowledge about VFS, I think that is practical as well, since we don't have sub-sub-dirs since all sub-dirs are negative dentries for empty dirs. And if casefold ioctl is "dir inode locked", I think that would be fine (?) I don't check the code though. Thanks, Gao Xiang > > - Eric >
