From: Al Viro <viro@zeniv.linux.org.uk>
To: Luis Henriques <luis@igalia.com>
Cc: Jeff Layton <jlayton@kernel.org>,
ceph-devel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
Ilya Dryomov <idryomov@gmail.com>
Subject: [PATCH 2/2] ceph: fix a race with rename() in ceph_mdsc_build_path()
Date: Sat, 15 Feb 2025 04:47:39 +0000 [thread overview]
Message-ID: <20250215044739.GB3451131@ZenIV> (raw)
In-Reply-To: <20250215044616.GF1977892@ZenIV>
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.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/ceph/caps.c | 18 +++++++-----------
fs/ceph/crypto.c | 20 +++-----------------
fs/ceph/crypto.h | 18 ++++--------------
fs/ceph/dir.c | 7 +++----
fs/ceph/mds_client.c | 4 ++--
5 files changed, 19 insertions(+), 48 deletions(-)
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index a8d8b56cf9d2..b1a8ff612c41 100644
--- 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 09346b91215a..76a4a7e60270 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -260,23 +260,17 @@ 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;
-
+ buf[elen] = '\0';
/* Handle the special case of snapshot names that start with '_' */
if (ceph_snap(dir) == CEPH_SNAPDIR && *p == '_') {
dir = parse_longname(parent, p, &name_len);
@@ -349,14 +343,6 @@ int ceph_encode_encrypted_dname(struct inode *parent, struct qstr *d_name,
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 d0768239a1c9..f752bbb2eb06 100644
--- 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 62e99e65250d..539954e2ea38 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -422,17 +422,16 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
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 54b3421501e9..7dc854631b55 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2766,8 +2766,8 @@ char *ceph_mdsc_build_path(struct ceph_mds_client *mdsc, struct dentry *dentry,
}
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);
--
2.39.5
next prev parent reply other threads:[~2025-02-15 4:47 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-14 2:47 [RFC] odd check in ceph_encode_encrypted_dname() Al Viro
2025-02-14 3:28 ` Al Viro
2025-02-14 14:05 ` Luis Henriques
2025-02-14 15:41 ` Jeff Layton
2025-02-14 16:05 ` Luis Henriques
2025-02-15 4:46 ` Al Viro
2025-02-15 4:47 ` [PATCH 1/2] prep for ceph_encode_encrypted_fname() fixes Al Viro
2025-02-15 12:41 ` Jeff Layton
2025-02-15 4:47 ` Al Viro [this message]
2025-02-15 12:42 ` [PATCH 2/2] ceph: fix a race with rename() in ceph_mdsc_build_path() Jeff Layton
2025-02-15 15:39 ` [RFC] odd check in ceph_encode_encrypted_dname() Luis Henriques
2025-02-17 17:56 ` Viacheslav Dubeyko
2025-02-17 18:48 ` Luis Henriques
2025-02-17 22:04 ` Viacheslav Dubeyko
2025-02-18 1:21 ` Al Viro
2025-02-18 23:52 ` Al Viro
2025-02-19 0:58 ` Viacheslav Dubeyko
2025-02-19 2:18 ` Al Viro
2025-02-19 23:22 ` Viacheslav Dubeyko
2025-02-21 1:21 ` Viacheslav Dubeyko
2025-02-14 15:30 ` Jeff Layton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250215044739.GB3451131@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=ceph-devel@vger.kernel.org \
--cc=idryomov@gmail.com \
--cc=jlayton@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=luis@igalia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).