Skip to content

Commit 3b7d93d

Browse files
aviallonidryomov
authored andcommitted
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): [<ffffffffaa68fb49>] __kmalloc_noprof+0x359/0x510 [<ffffffffc32bf1df>] ceph_mds_check_access+0x5bf/0x14e0 [ceph] [<ffffffffc3235722>] ceph_open+0x312/0xd80 [ceph] [<ffffffffaa7dd786>] do_dentry_open+0x456/0x1120 [<ffffffffaa7e3729>] vfs_open+0x79/0x360 [<ffffffffaa832875>] path_openat+0x1de5/0x4390 [<ffffffffaa834fcc>] do_filp_open+0x19c/0x3c0 [<ffffffffaa7e44a1>] do_sys_openat2+0x141/0x180 [<ffffffffaa7e4945>] __x64_sys_open+0xe5/0x1a0 [<ffffffffac2cc2f7>] do_syscall_64+0xb7/0x210 [<ffffffffac400130>] 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=<hidden>,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: 596afb0 ("ceph: add ceph_mds_check_access() helper") Signed-off-by: Antoine Viallon <antoine@lesviallon.fr> Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com> Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
1 parent 5bc55a3 commit 3b7d93d

File tree

1 file changed

+8
-8
lines changed

1 file changed

+8
-8
lines changed

fs/ceph/mds_client.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5690,18 +5690,18 @@ static int ceph_mds_auth_match(struct ceph_mds_client *mdsc,
56905690
*
56915691
* All the other cases --> mismatch
56925692
*/
5693+
bool path_matched = true;
56935694
char *first = strstr(_tpath, auth->match.path);
5694-
if (first != _tpath) {
5695-
if (free_tpath)
5696-
kfree(_tpath);
5697-
return 0;
5695+
if (first != _tpath ||
5696+
(tlen > len && _tpath[len] != '/')) {
5697+
path_matched = false;
56985698
}
56995699

5700-
if (tlen > len && _tpath[len] != '/') {
5701-
if (free_tpath)
5702-
kfree(_tpath);
5700+
if (free_tpath)
5701+
kfree(_tpath);
5702+
5703+
if (!path_matched)
57035704
return 0;
5704-
}
57055705
}
57065706
}
57075707

0 commit comments

Comments
 (0)