Jeff Layton <jlayton@xxxxxxxxxx> writes: > On Fri, 2022-03-04 at 16:14 +0000, Luís Henriques wrote: >> Since filenames in encrypted directories are already encrypted and shown >> as a base64-encoded string when the directory is locked, snapshot names >> should show a similar behaviour. >> >> Signed-off-by: Luís Henriques <lhenriques@xxxxxxx> >> --- >> fs/ceph/dir.c | 9 +++++++++ >> fs/ceph/inode.c | 13 +++++++++++++ >> 2 files changed, 22 insertions(+) >> >> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c >> index 934402f5e9e6..17d2f18a1fd1 100644 >> --- a/fs/ceph/dir.c >> +++ b/fs/ceph/dir.c >> @@ -1069,6 +1069,15 @@ static int ceph_mkdir(struct user_namespace *mnt_userns, struct inode *dir, >> op = CEPH_MDS_OP_MKSNAP; >> dout("mksnap dir %p snap '%pd' dn %p\n", dir, >> dentry, dentry); >> + /* >> + * Encrypted snapshots require d_revalidate to force a >> + * LOOKUPSNAP to cleanup dcache >> + */ >> + if (IS_ENCRYPTED(dir)) { >> + spin_lock(&dentry->d_lock); >> + dentry->d_flags |= DCACHE_NOKEY_NAME; >> + spin_unlock(&dentry->d_lock); >> + } >> } else if (ceph_snap(dir) == CEPH_NOSNAP) { >> dout("mkdir dir %p dn %p mode 0%ho\n", dir, dentry, mode); >> op = CEPH_MDS_OP_MKDIR; >> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c >> index 8b0832271fdf..357335a11384 100644 >> --- a/fs/ceph/inode.c >> +++ b/fs/ceph/inode.c >> @@ -182,6 +182,19 @@ struct inode *ceph_get_snapdir(struct inode *parent) >> ci->i_rbytes = 0; >> ci->i_btime = ceph_inode(parent)->i_btime; >> >> + /* if encrypted, just borrow fscrypt_auth from parent */ >> + if (IS_ENCRYPTED(parent)) { >> + struct ceph_inode_info *pci = ceph_inode(parent); >> + >> + ci->fscrypt_auth = kmemdup(pci->fscrypt_auth, >> + pci->fscrypt_auth_len, >> + GFP_KERNEL); >> + if (ci->fscrypt_auth) { >> + inode->i_flags |= S_ENCRYPTED; >> + ci->fscrypt_auth_len = pci->fscrypt_auth_len; >> + } else >> + dout("Failed to alloc memory for fscrypt_auth in snapdir\n"); > > Should we return an error in this case? Yeah, definitely. I'll add that and send out v2. Thanks. Cheers, -- Luís > >> + } >> if (inode->i_state & I_NEW) { >> inode->i_op = &ceph_snapdir_iops; >> inode->i_fop = &ceph_snapdir_fops; > > Seems simple and straightforward at first glance. > -- > Jeff Layton <jlayton@xxxxxxxxxx>