From 3b7d93db450e9d8ead80d75e2a303248f1528c35 Mon Sep 17 00:00:00 2001 From: Antoine Viallon Date: Tue, 14 Jan 2025 23:45:14 +0100 Subject: [PATCH 1/3] ceph: fix memory leak in ceph_mds_auth_match() We now free the temporary target path substring allocation on every possible branch, instead of omitting the default branch. In some cases, a memory leak occured, which could rapidly crash the system (depending on how many file accesses were attempted). This was detected in production because it caused a continuous memory growth, eventually triggering kernel OOM and completely hard-locking the kernel. Relevant kmemleak stacktrace: unreferenced object 0xffff888131e69900 (size 128): comm "git", pid 66104, jiffies 4295435999 hex dump (first 32 bytes): 76 6f 6c 75 6d 65 73 2f 63 6f 6e 74 61 69 6e 65 volumes/containe 72 73 2f 67 69 74 65 61 2f 67 69 74 65 61 2f 67 rs/gitea/gitea/g backtrace (crc 2f3bb450): [] __kmalloc_noprof+0x359/0x510 [] ceph_mds_check_access+0x5bf/0x14e0 [ceph] [] ceph_open+0x312/0xd80 [ceph] [] do_dentry_open+0x456/0x1120 [] vfs_open+0x79/0x360 [] path_openat+0x1de5/0x4390 [] do_filp_open+0x19c/0x3c0 [] do_sys_openat2+0x141/0x180 [] __x64_sys_open+0xe5/0x1a0 [] do_syscall_64+0xb7/0x210 [] entry_SYSCALL_64_after_hwframe+0x77/0x7f It can be triggered by mouting a subdirectory of a CephFS filesystem, and then trying to access files on this subdirectory with an auth token using a path-scoped capability: $ ceph auth get client.services [client.services] key = REDACTED caps mds = "allow rw fsname=cephfs path=/volumes/" caps mon = "allow r fsname=cephfs" caps osd = "allow rw tag cephfs data=cephfs" $ cat /proc/self/mounts services@[REDACTED].cephfs=/volumes/containers /ceph/containers ceph rw,noatime,name=services,secret=,ms_mode=prefer-crc,mount_timeout=300,acl,mon_addr=[REDACTED]:3300,recover_session=clean 0 0 $ seq 1 1000000 | xargs -P32 --replace={} touch /ceph/containers/file-{} && \ seq 1 1000000 | xargs -P32 --replace={} cat /ceph/containers/file-{} [ idryomov: combine if statements, rename rc to path_matched and make it a bool, formatting ] Cc: stable@vger.kernel.org Fixes: 596afb0b8933 ("ceph: add ceph_mds_check_access() helper") Signed-off-by: Antoine Viallon Reviewed-by: Viacheslav Dubeyko Signed-off-by: Ilya Dryomov --- fs/ceph/mds_client.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c index 785fe489ef4b..ae37f0e24c99 100644 --- a/fs/ceph/mds_client.c +++ b/fs/ceph/mds_client.c @@ -5690,18 +5690,18 @@ static int ceph_mds_auth_match(struct ceph_mds_client *mdsc, * * All the other cases --> mismatch */ + bool path_matched = true; char *first = strstr(_tpath, auth->match.path); - if (first != _tpath) { - if (free_tpath) - kfree(_tpath); - return 0; + if (first != _tpath || + (tlen > len && _tpath[len] != '/')) { + path_matched = false; } - if (tlen > len && _tpath[len] != '/') { - if (free_tpath) - kfree(_tpath); + if (free_tpath) + kfree(_tpath); + + if (!path_matched) return 0; - } } } From 2f0805d7c08bea71c95561bfb3e45d93b05196b9 Mon Sep 17 00:00:00 2001 From: Liang Jie Date: Fri, 10 Jan 2025 18:05:24 +0800 Subject: [PATCH 2/3] ceph: streamline request head structures in MDS client The existence of the ceph_mds_request_head_old structure in the MDS client code is no longer required due to improvements in handling different MDS request header versions. This patch removes the now redundant ceph_mds_request_head_old structure and replaces its usage with the flexible and extensible ceph_mds_request_head structure. Changes include: - Modification of find_legacy_request_head to directly cast the pointer to ceph_mds_request_head_legacy without going through the old structure. - Update sizeof calculations in create_request_message to use offsetofend for consistency and future-proofing, rather than referencing the old structure. - Use of the structured ceph_mds_request_head directly instead of the old one. Additionally, this consolidation normalizes the handling of request_head_version v1 to align with versions v2 and v3, leading to a more consistent and maintainable codebase. These changes simplify the codebase and reduce potential confusion stemming from the existence of an obsolete structure. Signed-off-by: Liang Jie Reviewed-by: Viacheslav Dubeyko Signed-off-by: Ilya Dryomov --- fs/ceph/mds_client.c | 16 ++++++++-------- include/linux/ceph/ceph_fs.h | 14 -------------- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c index ae37f0e24c99..921f08a27dd7 100644 --- a/fs/ceph/mds_client.c +++ b/fs/ceph/mds_client.c @@ -2945,12 +2945,12 @@ static struct ceph_mds_request_head_legacy * find_legacy_request_head(void *p, u64 features) { bool legacy = !(features & CEPH_FEATURE_FS_BTIME); - struct ceph_mds_request_head_old *ohead; + struct ceph_mds_request_head *head; if (legacy) return (struct ceph_mds_request_head_legacy *)p; - ohead = (struct ceph_mds_request_head_old *)p; - return (struct ceph_mds_request_head_legacy *)&ohead->oldest_client_tid; + head = (struct ceph_mds_request_head *)p; + return (struct ceph_mds_request_head_legacy *)&head->oldest_client_tid; } /* @@ -3020,7 +3020,7 @@ static struct ceph_msg *create_request_message(struct ceph_mds_session *session, if (legacy) len = sizeof(struct ceph_mds_request_head_legacy); else if (request_head_version == 1) - len = sizeof(struct ceph_mds_request_head_old); + len = offsetofend(struct ceph_mds_request_head, args); else if (request_head_version == 2) len = offsetofend(struct ceph_mds_request_head, ext_num_fwd); else @@ -3104,11 +3104,11 @@ static struct ceph_msg *create_request_message(struct ceph_mds_session *session, msg->hdr.version = cpu_to_le16(3); p = msg->front.iov_base + sizeof(*lhead); } else if (request_head_version == 1) { - struct ceph_mds_request_head_old *ohead = msg->front.iov_base; + struct ceph_mds_request_head *nhead = msg->front.iov_base; msg->hdr.version = cpu_to_le16(4); - ohead->version = cpu_to_le16(1); - p = msg->front.iov_base + sizeof(*ohead); + nhead->version = cpu_to_le16(1); + p = msg->front.iov_base + offsetofend(struct ceph_mds_request_head, args); } else if (request_head_version == 2) { struct ceph_mds_request_head *nhead = msg->front.iov_base; @@ -3265,7 +3265,7 @@ static int __prepare_send_request(struct ceph_mds_session *session, * so we limit to retry at most 256 times. */ if (req->r_attempts) { - old_max_retry = sizeof_field(struct ceph_mds_request_head_old, + old_max_retry = sizeof_field(struct ceph_mds_request_head, num_retry); old_max_retry = 1 << (old_max_retry * BITS_PER_BYTE); if ((old_version && req->r_attempts >= old_max_retry) || diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h index 2d7d86f0290d..c7f2c63b3bc3 100644 --- a/include/linux/ceph/ceph_fs.h +++ b/include/linux/ceph/ceph_fs.h @@ -504,20 +504,6 @@ struct ceph_mds_request_head_legacy { #define CEPH_MDS_REQUEST_HEAD_VERSION 3 -struct ceph_mds_request_head_old { - __le16 version; /* struct version */ - __le64 oldest_client_tid; - __le32 mdsmap_epoch; /* on client */ - __le32 flags; /* CEPH_MDS_FLAG_* */ - __u8 num_retry, num_fwd; /* count retry, fwd attempts */ - __le16 num_releases; /* # include cap/lease release records */ - __le32 op; /* mds op code */ - __le32 caller_uid, caller_gid; - __le64 ino; /* use this ino for openc, mkdir, mknod, - etc. (if replaying) */ - union ceph_mds_request_args_ext args; -} __attribute__ ((packed)); - struct ceph_mds_request_head { __le16 version; /* struct version */ __le64 oldest_client_tid; From 3981be13ec1baf811bfb93ed6a98bafc85cdeab1 Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Fri, 24 Jan 2025 11:46:23 -0800 Subject: [PATCH 3/3] ceph: exchange hardcoded value on NAME_MAX Initially, ceph_fs_debugfs_init() had temporary name buffer with hardcoded length of 80 symbols. Then, it was hardcoded again for 100 symbols. Finally, it makes sense to exchange hardcoded value on properly defined constant and 255 symbols should be enough for any name case. Signed-off-by: Viacheslav Dubeyko Reviewed-by: Patrick Donnelly Signed-off-by: Ilya Dryomov --- fs/ceph/debugfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c index fdf9dc15eafa..fdd404fc8112 100644 --- a/fs/ceph/debugfs.c +++ b/fs/ceph/debugfs.c @@ -412,7 +412,7 @@ void ceph_fs_debugfs_cleanup(struct ceph_fs_client *fsc) void ceph_fs_debugfs_init(struct ceph_fs_client *fsc) { - char name[100]; + char name[NAME_MAX]; doutc(fsc->client, "begin\n"); fsc->debugfs_congestion_kb =