ceph: fix a race with rename() in ceph_mdsc_build_path()

Lift copying the name into callers of ceph_encode_encrypted_dname()
that do not have it already copied; ceph_encode_encrypted_fname()
disappears.

That fixes a UAF in ceph_mdsc_build_path() - while the initial copy
of plaintext into buf is done under ->d_lock, we access the
original name again in ceph_encode_encrypted_fname() and that is
done without any locking.  With ceph_encode_encrypted_dname() using
the stable copy the problem goes away.

Tested-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index a8d8b56cf9d2113cfaeec9ae681cc486b68d1afa..b1a8ff612c41dc30e39db79190bba1d6448417bf 100644 (file)
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -4957,24 +4957,20 @@ int ceph_encode_dentry_release(void **p, struct dentry *dentry,
        cl = ceph_inode_to_client(dir);
        spin_lock(&dentry->d_lock);
        if (ret && di->lease_session && di->lease_session->s_mds == mds) {
+               int len = dentry->d_name.len;
                doutc(cl, "%p mds%d seq %d\n",  dentry, mds,
                      (int)di->lease_seq);
                rel->dname_seq = cpu_to_le32(di->lease_seq);
                __ceph_mdsc_drop_dentry_lease(dentry);
+               memcpy(*p, dentry->d_name.name, len);
                spin_unlock(&dentry->d_lock);
                if (IS_ENCRYPTED(dir) && fscrypt_has_encryption_key(dir)) {
-                       int ret2 = ceph_encode_encrypted_fname(dir, dentry, *p);
-
-                       if (ret2 < 0)
-                               return ret2;
-
-                       rel->dname_len = cpu_to_le32(ret2);
-                       *p += ret2;
-               } else {
-                       rel->dname_len = cpu_to_le32(dentry->d_name.len);
-                       memcpy(*p, dentry->d_name.name, dentry->d_name.len);
-                       *p += dentry->d_name.len;
+                       len = ceph_encode_encrypted_dname(dir, *p, len);
+                       if (len < 0)
+                               return len;
                }
+               rel->dname_len = cpu_to_le32(len);
+               *p += len;
        } else {
                spin_unlock(&dentry->d_lock);
        }

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index 2aef56fc6275434c23dfef7237a64bebfe4fae74..e312f52f48e4591daad215f9cd160447833f5b69 100644 (file)
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -253,23 +253,16 @@ static struct inode *parse_longname(const struct inode *parent,
        return dir;
 }
 
-int ceph_encode_encrypted_dname(struct inode *parent, struct qstr *d_name,
-                               char *buf)
+int ceph_encode_encrypted_dname(struct inode *parent, char *buf, int elen)
 {
        struct ceph_client *cl = ceph_inode_to_client(parent);
        struct inode *dir = parent;
        char *p = buf;
        u32 len;
-       int name_len;
-       int elen;
+       int name_len = elen;
        int ret;
        u8 *cryptbuf = NULL;
 
-       memcpy(buf, d_name->name, d_name->len);
-       elen = d_name->len;
-
-       name_len = elen;
-
        /* Handle the special case of snapshot names that start with '_' */
        if (ceph_snap(dir) == CEPH_SNAPDIR && *p == '_') {
                dir = parse_longname(parent, p, &name_len);
@@ -342,14 +335,6 @@ out:
        return elen;
 }
 
-int ceph_encode_encrypted_fname(struct inode *parent, struct dentry *dentry,
-                               char *buf)
-{
-       WARN_ON_ONCE(!fscrypt_has_encryption_key(parent));
-
-       return ceph_encode_encrypted_dname(parent, &dentry->d_name, buf);
-}
-
 /**
  * ceph_fname_to_usr - convert a filename for userland presentation
  * @fname: ceph_fname to be converted

diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
index d0768239a1c9c96bc0b992b6160f334e19a47f89..f752bbb2eb06cc2d53fa7182321ddbf65468ce57 100644 (file)
--- a/fs/ceph/crypto.h
+++ b/fs/ceph/crypto.h
@@ -102,10 +102,7 @@ int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
                                 struct ceph_acl_sec_ctx *as);
 void ceph_fscrypt_as_ctx_to_req(struct ceph_mds_request *req,
                                struct ceph_acl_sec_ctx *as);
-int ceph_encode_encrypted_dname(struct inode *parent, struct qstr *d_name,
-                               char *buf);
-int ceph_encode_encrypted_fname(struct inode *parent, struct dentry *dentry,
-                               char *buf);
+int ceph_encode_encrypted_dname(struct inode *parent, char *buf, int len);
 
 static inline int ceph_fname_alloc_buffer(struct inode *parent,
                                          struct fscrypt_str *fname)
@@ -194,17 +191,10 @@ static inline void ceph_fscrypt_as_ctx_to_req(struct ceph_mds_request *req,
 {
 }
 
-static inline int ceph_encode_encrypted_dname(struct inode *parent,
-                                             struct qstr *d_name, char *buf)
+static inline int ceph_encode_encrypted_dname(struct inode *parent, char *buf,
+                                             int len)
 {
-       memcpy(buf, d_name->name, d_name->len);
-       return d_name->len;
-}
-
-static inline int ceph_encode_encrypted_fname(struct inode *parent,
-                                             struct dentry *dentry, char *buf)
-{
-       return -EOPNOTSUPP;
+       return len;
 }
 
 static inline int ceph_fname_alloc_buffer(struct inode *parent,

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index a321aa6d0ed22680fbfd1c2939c7da47e8607d26..8478e7e75df66c19787ff74e5ecda915622618e0 100644 (file)
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -423,17 +423,16 @@ more:
                        req->r_inode_drop = CEPH_CAP_FILE_EXCL;
                }
                if (dfi->last_name) {
-                       struct qstr d_name = { .name = dfi->last_name,
-                                              .len = strlen(dfi->last_name) };
+                       int len = strlen(dfi->last_name);
 
                        req->r_path2 = kzalloc(NAME_MAX + 1, GFP_KERNEL);
                        if (!req->r_path2) {
                                ceph_mdsc_put_request(req);
                                return -ENOMEM;
                        }
+                       memcpy(req->r_path2, dfi->last_name, len);
 
-                       err = ceph_encode_encrypted_dname(inode, &d_name,
-                                                         req->r_path2);
+                       err = ceph_encode_encrypted_dname(inode, req->r_path2, len);
                        if (err < 0) {
                                ceph_mdsc_put_request(req);
                                return err;

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 230e0c3f341f7119db7a189a5a378772cc4250cd..0f497c39ff8246d755e01ef5df4b6db0af8f8352 100644 (file)
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2766,8 +2766,8 @@ retry:
                        }
 
                        if (fscrypt_has_encryption_key(d_inode(parent))) {
-                               len = ceph_encode_encrypted_fname(d_inode(parent),
-                                                                 cur, buf);
+                               len = ceph_encode_encrypted_dname(d_inode(parent),
+                                                                 buf, len);
                                if (len < 0) {
                                        dput(parent);
                                        dput(cur);