Al and syzbot reported that 4fbc0c711b24 (ceph: remove the extra slashes in the server path) had caused a regression where an allocation could be done under spinlock. Fix this by keeping a canonicalized version of the path in the mount options. Then we can simply compare those without making copies at all during the comparison. Fixes: 4fbc0c711b24 ("ceph: remove the extra slashes in the server path") Reported-by: syzbot+98704a51af8e3d9425a9@xxxxxxxxxxxxxxxxxxxxxxxxx Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> --- fs/ceph/super.c | 170 ++++++++++++++++++++++-------------------------- fs/ceph/super.h | 1 + 2 files changed, 79 insertions(+), 92 deletions(-) diff --git a/fs/ceph/super.c b/fs/ceph/super.c index 5fa28e98d2b8..196d547c7054 100644 --- a/fs/ceph/super.c +++ b/fs/ceph/super.c @@ -208,6 +208,69 @@ struct ceph_parse_opts_ctx { struct ceph_mount_options *opts; }; +/** + * canonicalize_path - Remove the extra slashes in the server path + * @server_path: the server path and could be NULL + * + * Return NULL if the path is NULL or only consists of "/", or a string + * without any extra slashes including the leading slash(es) and the + * slash(es) at the end of the server path, such as: + * "//dir1////dir2///" --> "dir1/dir2" + */ +static char *canonicalize_path(const char *server_path) +{ + const char *path = server_path; + const char *cur, *end; + char *buf, *p; + int len; + + /* remove all the leading slashes */ + while (*path == '/') + path++; + + /* if the server path only consists of slashes */ + if (*path == '\0') + return NULL; + + len = strlen(path); + + buf = kmalloc(len + 1, GFP_KERNEL); + if (!buf) + return ERR_PTR(-ENOMEM); + + end = path + len; + p = buf; + do { + cur = strchr(path, '/'); + if (!cur) + cur = end; + + len = cur - path; + + /* including one '/' */ + if (cur != end) + len += 1; + + memcpy(p, path, len); + p += len; + + while (cur <= end && *cur == '/') + cur++; + path = cur; + } while (path < end); + + *p = '\0'; + + /* + * remove the last slash if there has and just to make sure that + * we will get something like "dir1/dir2" + */ + if (*(--p) == '/') + *p = '\0'; + + return buf; +} + /* * Parse the source parameter. Distinguish the server list from the path. * @@ -230,15 +293,23 @@ static int ceph_parse_source(struct fs_parameter *param, struct fs_context *fc) dev_name_end = strchr(dev_name, '/'); if (dev_name_end) { - kfree(fsopt->server_path); /* * The server_path will include the whole chars from userland * including the leading '/'. */ + kfree(fsopt->server_path); fsopt->server_path = kstrdup(dev_name_end, GFP_KERNEL); if (!fsopt->server_path) return -ENOMEM; + + kfree(fsopt->canon_path); + fsopt->canon_path = canonicalize_path(fsopt->server_path); + if (fsopt->canon_path && IS_ERR(fsopt->canon_path)) { + ret = PTR_ERR(fsopt->canon_path); + fsopt->canon_path = NULL; + return ret; + } } else { dev_name_end = dev_name + strlen(dev_name); } @@ -447,6 +518,7 @@ static void destroy_mount_options(struct ceph_mount_options *args) kfree(args->snapdir_name); kfree(args->mds_namespace); kfree(args->server_path); + kfree(args->canon_path); kfree(args->fscache_uniq); kfree(args); } @@ -462,73 +534,6 @@ static int strcmp_null(const char *s1, const char *s2) return strcmp(s1, s2); } -/** - * path_remove_extra_slash - Remove the extra slashes in the server path - * @server_path: the server path and could be NULL - * - * Return NULL if the path is NULL or only consists of "/", or a string - * without any extra slashes including the leading slash(es) and the - * slash(es) at the end of the server path, such as: - * "//dir1////dir2///" --> "dir1/dir2" - */ -static char *path_remove_extra_slash(const char *server_path) -{ - const char *path = server_path; - const char *cur, *end; - char *buf, *p; - int len; - - /* if the server path is omitted */ - if (!path) - return NULL; - - /* remove all the leading slashes */ - while (*path == '/') - path++; - - /* if the server path only consists of slashes */ - if (*path == '\0') - return NULL; - - len = strlen(path); - - buf = kmalloc(len + 1, GFP_KERNEL); - if (!buf) - return ERR_PTR(-ENOMEM); - - end = path + len; - p = buf; - do { - cur = strchr(path, '/'); - if (!cur) - cur = end; - - len = cur - path; - - /* including one '/' */ - if (cur != end) - len += 1; - - memcpy(p, path, len); - p += len; - - while (cur <= end && *cur == '/') - cur++; - path = cur; - } while (path < end); - - *p = '\0'; - - /* - * remove the last slash if there has and just to make sure that - * we will get something like "dir1/dir2" - */ - if (*(--p) == '/') - *p = '\0'; - - return buf; -} - static int compare_mount_options(struct ceph_mount_options *new_fsopt, struct ceph_options *new_opt, struct ceph_fs_client *fsc) @@ -536,7 +541,6 @@ static int compare_mount_options(struct ceph_mount_options *new_fsopt, struct ceph_mount_options *fsopt1 = new_fsopt; struct ceph_mount_options *fsopt2 = fsc->mount_options; int ofs = offsetof(struct ceph_mount_options, snapdir_name); - char *p1, *p2; int ret; ret = memcmp(fsopt1, fsopt2, ofs); @@ -546,21 +550,12 @@ static int compare_mount_options(struct ceph_mount_options *new_fsopt, ret = strcmp_null(fsopt1->snapdir_name, fsopt2->snapdir_name); if (ret) return ret; + ret = strcmp_null(fsopt1->mds_namespace, fsopt2->mds_namespace); if (ret) return ret; - p1 = path_remove_extra_slash(fsopt1->server_path); - if (IS_ERR(p1)) - return PTR_ERR(p1); - p2 = path_remove_extra_slash(fsopt2->server_path); - if (IS_ERR(p2)) { - kfree(p1); - return PTR_ERR(p2); - } - ret = strcmp_null(p1, p2); - kfree(p1); - kfree(p2); + ret = strcmp_null(fsopt1->canon_path, fsopt2->canon_path); if (ret) return ret; @@ -963,7 +958,9 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc, mutex_lock(&fsc->client->mount_mutex); if (!fsc->sb->s_root) { - const char *path, *p; + const char *path = fsc->mount_options->canon_path ? + fsc->mount_options->canon_path : ""; + err = __ceph_open_session(fsc->client, started); if (err < 0) goto out; @@ -975,22 +972,11 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc, goto out; } - p = path_remove_extra_slash(fsc->mount_options->server_path); - if (IS_ERR(p)) { - err = PTR_ERR(p); - goto out; - } - /* if the server path is omitted or just consists of '/' */ - if (!p) - path = ""; - else - path = p; dout("mount opening path '%s'\n", path); ceph_fs_debugfs_init(fsc); root = open_root_dentry(fsc, path, started); - kfree(p); if (IS_ERR(root)) { err = PTR_ERR(root); goto out; diff --git a/fs/ceph/super.h b/fs/ceph/super.h index e8f891770f9d..70aa32cfb64d 100644 --- a/fs/ceph/super.h +++ b/fs/ceph/super.h @@ -94,6 +94,7 @@ struct ceph_mount_options { char *snapdir_name; /* default ".snap" */ char *mds_namespace; /* default NULL */ char *server_path; /* default "/" */ + char *canon_path; /* default "/" */ char *fscache_uniq; /* default NULL */ }; -- 2.24.1