| Message ID | 20220304161403.19295-4-lhenriques@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: minor fixes and encrypted snapshot names | expand |
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@suse.de> > --- > 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? > + } > 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.
On 3/5/22 12:14 AM, 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@suse.de> > --- > 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); > + } Yeah, this fix looks cool. > } 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"); > + } > if (inode->i_state & I_NEW) { > inode->i_op = &ceph_snapdir_iops; > inode->i_fop = &ceph_snapdir_fops; >
Jeff Layton <jlayton@kernel.org> 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@suse.de> >> --- >> 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,
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"); + } if (inode->i_state & I_NEW) { inode->i_op = &ceph_snapdir_iops; inode->i_fop = &ceph_snapdir_fops;
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@suse.de> --- fs/ceph/dir.c | 9 +++++++++ fs/ceph/inode.c | 13 +++++++++++++ 2 files changed, 22 insertions(+)