Skip to content

Commit 0d2da25

Browse files
author
Al Viro
committed
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>
1 parent 28032ef commit 0d2da25

5 files changed

Lines changed: 18 additions & 48 deletions

File tree

fs/ceph/caps.c

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4957,24 +4957,20 @@ int ceph_encode_dentry_release(void **p, struct dentry *dentry,
49574957
cl = ceph_inode_to_client(dir);
49584958
spin_lock(&dentry->d_lock);
49594959
if (ret && di->lease_session && di->lease_session->s_mds == mds) {
4960+
int len = dentry->d_name.len;
49604961
doutc(cl, "%p mds%d seq %d\n", dentry, mds,
49614962
(int)di->lease_seq);
49624963
rel->dname_seq = cpu_to_le32(di->lease_seq);
49634964
__ceph_mdsc_drop_dentry_lease(dentry);
4965+
memcpy(*p, dentry->d_name.name, len);
49644966
spin_unlock(&dentry->d_lock);
49654967
if (IS_ENCRYPTED(dir) && fscrypt_has_encryption_key(dir)) {
4966-
int ret2 = ceph_encode_encrypted_fname(dir, dentry, *p);
4967-
4968-
if (ret2 < 0)
4969-
return ret2;
4970-
4971-
rel->dname_len = cpu_to_le32(ret2);
4972-
*p += ret2;
4973-
} else {
4974-
rel->dname_len = cpu_to_le32(dentry->d_name.len);
4975-
memcpy(*p, dentry->d_name.name, dentry->d_name.len);
4976-
*p += dentry->d_name.len;
4968+
len = ceph_encode_encrypted_dname(dir, *p, len);
4969+
if (len < 0)
4970+
return len;
49774971
}
4972+
rel->dname_len = cpu_to_le32(len);
4973+
*p += len;
49784974
} else {
49794975
spin_unlock(&dentry->d_lock);
49804976
}

fs/ceph/crypto.c

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -253,23 +253,16 @@ static struct inode *parse_longname(const struct inode *parent,
253253
return dir;
254254
}
255255

256-
int ceph_encode_encrypted_dname(struct inode *parent, struct qstr *d_name,
257-
char *buf)
256+
int ceph_encode_encrypted_dname(struct inode *parent, char *buf, int elen)
258257
{
259258
struct ceph_client *cl = ceph_inode_to_client(parent);
260259
struct inode *dir = parent;
261260
char *p = buf;
262261
u32 len;
263-
int name_len;
264-
int elen;
262+
int name_len = elen;
265263
int ret;
266264
u8 *cryptbuf = NULL;
267265

268-
memcpy(buf, d_name->name, d_name->len);
269-
elen = d_name->len;
270-
271-
name_len = elen;
272-
273266
/* Handle the special case of snapshot names that start with '_' */
274267
if (ceph_snap(dir) == CEPH_SNAPDIR && *p == '_') {
275268
dir = parse_longname(parent, p, &name_len);
@@ -342,14 +335,6 @@ int ceph_encode_encrypted_dname(struct inode *parent, struct qstr *d_name,
342335
return elen;
343336
}
344337

345-
int ceph_encode_encrypted_fname(struct inode *parent, struct dentry *dentry,
346-
char *buf)
347-
{
348-
WARN_ON_ONCE(!fscrypt_has_encryption_key(parent));
349-
350-
return ceph_encode_encrypted_dname(parent, &dentry->d_name, buf);
351-
}
352-
353338
/**
354339
* ceph_fname_to_usr - convert a filename for userland presentation
355340
* @fname: ceph_fname to be converted

fs/ceph/crypto.h

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,7 @@ int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
102102
struct ceph_acl_sec_ctx *as);
103103
void ceph_fscrypt_as_ctx_to_req(struct ceph_mds_request *req,
104104
struct ceph_acl_sec_ctx *as);
105-
int ceph_encode_encrypted_dname(struct inode *parent, struct qstr *d_name,
106-
char *buf);
107-
int ceph_encode_encrypted_fname(struct inode *parent, struct dentry *dentry,
108-
char *buf);
105+
int ceph_encode_encrypted_dname(struct inode *parent, char *buf, int len);
109106

110107
static inline int ceph_fname_alloc_buffer(struct inode *parent,
111108
struct fscrypt_str *fname)
@@ -194,17 +191,10 @@ static inline void ceph_fscrypt_as_ctx_to_req(struct ceph_mds_request *req,
194191
{
195192
}
196193

197-
static inline int ceph_encode_encrypted_dname(struct inode *parent,
198-
struct qstr *d_name, char *buf)
194+
static inline int ceph_encode_encrypted_dname(struct inode *parent, char *buf,
195+
int len)
199196
{
200-
memcpy(buf, d_name->name, d_name->len);
201-
return d_name->len;
202-
}
203-
204-
static inline int ceph_encode_encrypted_fname(struct inode *parent,
205-
struct dentry *dentry, char *buf)
206-
{
207-
return -EOPNOTSUPP;
197+
return len;
208198
}
209199

210200
static inline int ceph_fname_alloc_buffer(struct inode *parent,

fs/ceph/dir.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -423,17 +423,16 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
423423
req->r_inode_drop = CEPH_CAP_FILE_EXCL;
424424
}
425425
if (dfi->last_name) {
426-
struct qstr d_name = { .name = dfi->last_name,
427-
.len = strlen(dfi->last_name) };
426+
int len = strlen(dfi->last_name);
428427

429428
req->r_path2 = kzalloc(NAME_MAX + 1, GFP_KERNEL);
430429
if (!req->r_path2) {
431430
ceph_mdsc_put_request(req);
432431
return -ENOMEM;
433432
}
433+
memcpy(req->r_path2, dfi->last_name, len);
434434

435-
err = ceph_encode_encrypted_dname(inode, &d_name,
436-
req->r_path2);
435+
err = ceph_encode_encrypted_dname(inode, req->r_path2, len);
437436
if (err < 0) {
438437
ceph_mdsc_put_request(req);
439438
return err;

fs/ceph/mds_client.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2766,8 +2766,8 @@ char *ceph_mdsc_build_path(struct ceph_mds_client *mdsc, struct dentry *dentry,
27662766
}
27672767

27682768
if (fscrypt_has_encryption_key(d_inode(parent))) {
2769-
len = ceph_encode_encrypted_fname(d_inode(parent),
2770-
cur, buf);
2769+
len = ceph_encode_encrypted_dname(d_inode(parent),
2770+
buf, len);
27712771
if (len < 0) {
27722772
dput(parent);
27732773
dput(cur);

0 commit comments

Comments
 (0)