All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] stable CephFS backports
@ 2020-04-08 10:58 ` Luis Henriques
  0 siblings, 0 replies; 14+ messages in thread
From: Luis Henriques @ 2020-04-08 10:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Sasha Levin
  Cc: Jeff Layton, Ilya Dryomov, ceph-devel, stable, Luis Henriques

Hi!

Please pick the backports for the following upstream commits:

  4fbc0c711b24 "ceph: remove the extra slashes in the server path"
  b27a939e8376 "ceph: canonicalize server path in place"

They fix an ancient bug that can be reproduced in kernels as old as 4.9 (I
couldn't reproduced it with 4.4).

Cheers,
--
Luis Henriques

Ilya Dryomov (1):
  ceph: canonicalize server path in place

Xiubo Li (1):
  ceph: remove the extra slashes in the server path

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 0/2] stable CephFS backports
@ 2020-04-08 10:58 ` Luis Henriques
  0 siblings, 0 replies; 14+ messages in thread
From: Luis Henriques @ 2020-04-08 10:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Sasha Levin
  Cc: Jeff Layton, Ilya Dryomov, ceph-devel, stable, Luis Henriques

Hi!

Please pick the backports for the following upstream commits:

  4fbc0c711b24 "ceph: remove the extra slashes in the server path"
  b27a939e8376 "ceph: canonicalize server path in place"

They fix an ancient bug that can be reproduced in kernels as old as 4.9 (I
couldn't reproduced it with 4.4).

Cheers,

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 4.9 1/2] ceph: remove the extra slashes in the server path
  2020-04-08 10:58 ` Luis Henriques
  (?)
@ 2020-04-08 10:58 ` Luis Henriques
  -1 siblings, 0 replies; 14+ messages in thread
From: Luis Henriques @ 2020-04-08 10:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Sasha Levin
  Cc: Jeff Layton, Ilya Dryomov, ceph-devel, stable, Xiubo Li, Luis Henriques

From: Xiubo Li <xiubli@redhat.com>

commit 4fbc0c711b2464ee1551850b85002faae0b775d5 upstream.

It's possible to pass the mount helper a server path that has more
than one contiguous slash character. For example:

  $ mount -t ceph 192.168.195.165:40176:/// /mnt/cephfs/

In the MDS server side the extra slashes of the server path will be
treated as snap dir, and then we can get the following debug logs:

  ceph:  mount opening path //
  ceph:  open_root_inode opening '//'
  ceph:  fill_trace 0000000059b8a3bc is_dentry 0 is_target 1
  ceph:  alloc_inode 00000000dc4ca00b
  ceph:  get_inode created new inode 00000000dc4ca00b 1.ffffffffffffffff ino 1
  ceph:  get_inode on 1=1.ffffffffffffffff got 00000000dc4ca00b

And then when creating any new file or directory under the mount
point, we can hit the following BUG_ON in ceph_fill_trace():

  BUG_ON(ceph_snap(dir) != dvino.snap);

Have the client ignore the extra slashes in the server path when
mounting. This will also canonicalize the path, so that identical mounts
can be consilidated.

1) "//mydir1///mydir//"
2) "/mydir1/mydir"
3) "/mydir1/mydir/"

Regardless of the internal treatment of these paths, the kernel still
stores the original string including the leading '/' for presentation
to userland.

URL: https://tracker.ceph.com/issues/42771
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Signed-off-by: Luis Henriques <lhenriques@suse.com>
---
 fs/ceph/super.c | 118 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 99 insertions(+), 19 deletions(-)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index c42cbd19ff05..20e62749bbb7 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -85,7 +85,6 @@ static int ceph_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return 0;
 }
 
-
 static int ceph_sync_fs(struct super_block *sb, int wait)
 {
 	struct ceph_fs_client *fsc = ceph_sb_to_client(sb);
@@ -321,6 +320,73 @@ 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)
@@ -328,6 +394,7 @@ 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);
@@ -341,7 +408,17 @@ static int compare_mount_options(struct ceph_mount_options *new_fsopt,
 	if (ret)
 		return ret;
 
-	ret = strcmp_null(fsopt1->server_path, fsopt2->server_path);
+	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);
 	if (ret)
 		return ret;
 
@@ -396,12 +473,14 @@ static int parse_mount_options(struct ceph_mount_options **pfsopt,
 	 */
 	dev_name_end = strchr(dev_name, '/');
 	if (dev_name_end) {
-		if (strlen(dev_name_end) > 1) {
-			fsopt->server_path = kstrdup(dev_name_end, GFP_KERNEL);
-			if (!fsopt->server_path) {
-				err = -ENOMEM;
-				goto out;
-			}
+		/*
+		 * The server_path will include the whole chars from userland
+		 * including the leading '/'.
+		 */
+		fsopt->server_path = kstrdup(dev_name_end, GFP_KERNEL);
+		if (!fsopt->server_path) {
+			err = -ENOMEM;
+			goto out;
 		}
 	} else {
 		dev_name_end = dev_name + strlen(dev_name);
@@ -725,7 +804,6 @@ static void destroy_caches(void)
 	ceph_fscache_unregister();
 }
 
-
 /*
  * ceph_umount_begin - initiate forced umount.  Tear down down the
  * mount, skipping steps that may hang while waiting for server(s).
@@ -812,9 +890,6 @@ static struct dentry *open_root_dentry(struct ceph_fs_client *fsc,
 	return root;
 }
 
-
-
-
 /*
  * mount: join the ceph cluster, and open root directory.
  */
@@ -828,24 +903,29 @@ 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;
+		const char *path, *p;
 		err = __ceph_open_session(fsc->client, started);
 		if (err < 0)
 			goto out;
 
-		if (!fsc->mount_options->server_path) {
-			path = "";
-			dout("mount opening path \\t\n");
-		} else {
-			path = fsc->mount_options->server_path + 1;
-			dout("mount opening path %s\n", path);
+		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);
 
 		err = ceph_fs_debugfs_init(fsc);
 		if (err < 0)
 			goto out;
 
 		root = open_root_dentry(fsc, path, started);
+		kfree(p);
 		if (IS_ERR(root)) {
 			err = PTR_ERR(root);
 			goto out;

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4.9 2/2] ceph: canonicalize server path in place
  2020-04-08 10:58 ` Luis Henriques
  (?)
  (?)
@ 2020-04-08 10:58 ` Luis Henriques
  -1 siblings, 0 replies; 14+ messages in thread
From: Luis Henriques @ 2020-04-08 10:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Sasha Levin
  Cc: Jeff Layton, Ilya Dryomov, ceph-devel, stable,
	syzbot+98704a51af8e3d9425a9, Luis Henriques

From: Ilya Dryomov <idryomov@gmail.com>

commit b27a939e8376a3f1ed09b9c33ef44d20f18ec3d0 upstream.

syzbot reported that 4fbc0c711b24 ("ceph: remove the extra slashes in
the server path") had caused a regression where an allocation could be
done under a spinlock -- compare_mount_options() is called by sget_fc()
with sb_lock held.

We don't really need the supplied server path, so canonicalize it
in place and compare it directly.  To make this work, the leading
slash is kept around and the logic in ceph_real_mount() to skip it
is restored.  CEPH_MSG_CLIENT_SESSION now reports the same (i.e.
canonicalized) path, with the leading slash of course.

Fixes: 4fbc0c711b24 ("ceph: remove the extra slashes in the server path")
Reported-by: syzbot+98704a51af8e3d9425a9@syzkaller.appspotmail.com
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Luis Henriques <lhenriques@suse.com>
---
 fs/ceph/super.c | 118 +++++++++++-------------------------------------
 fs/ceph/super.h |   2 +-
 2 files changed, 28 insertions(+), 92 deletions(-)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 20e62749bbb7..ec1640f3167b 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -177,6 +177,26 @@ static match_table_t fsopt_tokens = {
 	{-1, NULL}
 };
 
+/*
+ * Remove adjacent slashes and then the trailing slash, unless it is
+ * the only remaining character.
+ *
+ * E.g. "//dir1////dir2///" --> "/dir1/dir2", "///" --> "/".
+ */
+static void canonicalize_path(char *path)
+{
+	int i, j = 0;
+
+	for (i = 0; path[i] != '\0'; i++) {
+		if (path[i] != '/' || j < 1 || path[j - 1] != '/')
+			path[j++] = path[i];
+	}
+
+	if (j > 1 && path[j - 1] == '/')
+		j--;
+	path[j] = '\0';
+}
+
 static int parse_fsopt_token(char *c, void *private)
 {
 	struct ceph_mount_options *fsopt = private;
@@ -320,73 +340,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)
@@ -394,7 +347,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);
@@ -404,21 +356,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->server_path, fsopt2->server_path);
 	if (ret)
 		return ret;
 
@@ -482,6 +425,8 @@ static int parse_mount_options(struct ceph_mount_options **pfsopt,
 			err = -ENOMEM;
 			goto out;
 		}
+
+		canonicalize_path(fsopt->server_path);
 	} else {
 		dev_name_end = dev_name + strlen(dev_name);
 	}
@@ -903,21 +848,13 @@ 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->server_path ?
+				     fsc->mount_options->server_path + 1 : "";
+
 		err = __ceph_open_session(fsc->client, started);
 		if (err < 0)
 			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);
 
 		err = ceph_fs_debugfs_init(fsc);
@@ -925,7 +862,6 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc)
 			goto out;
 
 		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 9bd0d928057b..9f18635f78c7 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -70,7 +70,7 @@ struct ceph_mount_options {
 
 	char *snapdir_name;   /* default ".snap" */
 	char *mds_namespace;  /* default NULL */
-	char *server_path;    /* default  "/" */
+	char *server_path;    /* default NULL (means "/") */
 };
 
 struct ceph_fs_client {

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4.14,4.19 1/2] ceph: remove the extra slashes in the server path
  2020-04-08 10:58 ` Luis Henriques
                   ` (2 preceding siblings ...)
  (?)
@ 2020-04-08 10:58 ` Luis Henriques
  -1 siblings, 0 replies; 14+ messages in thread
From: Luis Henriques @ 2020-04-08 10:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Sasha Levin
  Cc: Jeff Layton, Ilya Dryomov, ceph-devel, stable, Xiubo Li, Luis Henriques

From: Xiubo Li <xiubli@redhat.com>

commit 4fbc0c711b2464ee1551850b85002faae0b775d5 upstream.

It's possible to pass the mount helper a server path that has more
than one contiguous slash character. For example:

  $ mount -t ceph 192.168.195.165:40176:/// /mnt/cephfs/

In the MDS server side the extra slashes of the server path will be
treated as snap dir, and then we can get the following debug logs:

  ceph:  mount opening path //
  ceph:  open_root_inode opening '//'
  ceph:  fill_trace 0000000059b8a3bc is_dentry 0 is_target 1
  ceph:  alloc_inode 00000000dc4ca00b
  ceph:  get_inode created new inode 00000000dc4ca00b 1.ffffffffffffffff ino 1
  ceph:  get_inode on 1=1.ffffffffffffffff got 00000000dc4ca00b

And then when creating any new file or directory under the mount
point, we can hit the following BUG_ON in ceph_fill_trace():

  BUG_ON(ceph_snap(dir) != dvino.snap);

Have the client ignore the extra slashes in the server path when
mounting. This will also canonicalize the path, so that identical mounts
can be consilidated.

1) "//mydir1///mydir//"
2) "/mydir1/mydir"
3) "/mydir1/mydir/"

Regardless of the internal treatment of these paths, the kernel still
stores the original string including the leading '/' for presentation
to userland.

URL: https://tracker.ceph.com/issues/42771
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Signed-off-by: Luis Henriques <lhenriques@suse.com>
---
 fs/ceph/super.c | 120 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 101 insertions(+), 19 deletions(-)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index c4314f449240..712e4a15cb3e 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -105,7 +105,6 @@ static int ceph_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return 0;
 }
 
-
 static int ceph_sync_fs(struct super_block *sb, int wait)
 {
 	struct ceph_fs_client *fsc = ceph_sb_to_client(sb);
@@ -399,6 +398,73 @@ 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)
@@ -406,6 +472,7 @@ 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);
@@ -418,9 +485,21 @@ static int compare_mount_options(struct ceph_mount_options *new_fsopt,
 	ret = strcmp_null(fsopt1->mds_namespace, fsopt2->mds_namespace);
 	if (ret)
 		return ret;
-	ret = strcmp_null(fsopt1->server_path, fsopt2->server_path);
+
+	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);
 	if (ret)
 		return ret;
+
 	ret = strcmp_null(fsopt1->fscache_uniq, fsopt2->fscache_uniq);
 	if (ret)
 		return ret;
@@ -476,12 +555,14 @@ static int parse_mount_options(struct ceph_mount_options **pfsopt,
 	 */
 	dev_name_end = strchr(dev_name, '/');
 	if (dev_name_end) {
-		if (strlen(dev_name_end) > 1) {
-			fsopt->server_path = kstrdup(dev_name_end, GFP_KERNEL);
-			if (!fsopt->server_path) {
-				err = -ENOMEM;
-				goto out;
-			}
+		/*
+		 * The server_path will include the whole chars from userland
+		 * including the leading '/'.
+		 */
+		fsopt->server_path = kstrdup(dev_name_end, GFP_KERNEL);
+		if (!fsopt->server_path) {
+			err = -ENOMEM;
+			goto out;
 		}
 	} else {
 		dev_name_end = dev_name + strlen(dev_name);
@@ -810,7 +891,6 @@ static void destroy_caches(void)
 	ceph_fscache_unregister();
 }
 
-
 /*
  * ceph_umount_begin - initiate forced umount.  Tear down down the
  * mount, skipping steps that may hang while waiting for server(s).
@@ -897,9 +977,6 @@ static struct dentry *open_root_dentry(struct ceph_fs_client *fsc,
 	return root;
 }
 
-
-
-
 /*
  * mount: join the ceph cluster, and open root directory.
  */
@@ -913,7 +990,7 @@ 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;
+		const char *path, *p;
 		err = __ceph_open_session(fsc->client, started);
 		if (err < 0)
 			goto out;
@@ -925,19 +1002,24 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc)
 				goto out;
 		}
 
-		if (!fsc->mount_options->server_path) {
-			path = "";
-			dout("mount opening path \\t\n");
-		} else {
-			path = fsc->mount_options->server_path + 1;
-			dout("mount opening path %s\n", path);
+		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);
 
 		err = ceph_fs_debugfs_init(fsc);
 		if (err < 0)
 			goto out;
 
 		root = open_root_dentry(fsc, path, started);
+		kfree(p);
 		if (IS_ERR(root)) {
 			err = PTR_ERR(root);
 			goto out;

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4.14,4.19 2/2] ceph: canonicalize server path in place
  2020-04-08 10:58 ` Luis Henriques
                   ` (3 preceding siblings ...)
  (?)
@ 2020-04-08 10:58 ` Luis Henriques
  -1 siblings, 0 replies; 14+ messages in thread
From: Luis Henriques @ 2020-04-08 10:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Sasha Levin
  Cc: Jeff Layton, Ilya Dryomov, ceph-devel, stable,
	syzbot+98704a51af8e3d9425a9, Luis Henriques

From: Ilya Dryomov <idryomov@gmail.com>

commit b27a939e8376a3f1ed09b9c33ef44d20f18ec3d0 upstream.

syzbot reported that 4fbc0c711b24 ("ceph: remove the extra slashes in
the server path") had caused a regression where an allocation could be
done under a spinlock -- compare_mount_options() is called by sget_fc()
with sb_lock held.

We don't really need the supplied server path, so canonicalize it
in place and compare it directly.  To make this work, the leading
slash is kept around and the logic in ceph_real_mount() to skip it
is restored.  CEPH_MSG_CLIENT_SESSION now reports the same (i.e.
canonicalized) path, with the leading slash of course.

Fixes: 4fbc0c711b24 ("ceph: remove the extra slashes in the server path")
Reported-by: syzbot+98704a51af8e3d9425a9@syzkaller.appspotmail.com
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Luis Henriques <lhenriques@suse.com>
---
 fs/ceph/super.c | 118 +++++++++++-------------------------------------
 fs/ceph/super.h |   2 +-
 2 files changed, 28 insertions(+), 92 deletions(-)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 712e4a15cb3e..18e967089aeb 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -205,6 +205,26 @@ static match_table_t fsopt_tokens = {
 	{-1, NULL}
 };
 
+/*
+ * Remove adjacent slashes and then the trailing slash, unless it is
+ * the only remaining character.
+ *
+ * E.g. "//dir1////dir2///" --> "/dir1/dir2", "///" --> "/".
+ */
+static void canonicalize_path(char *path)
+{
+	int i, j = 0;
+
+	for (i = 0; path[i] != '\0'; i++) {
+		if (path[i] != '/' || j < 1 || path[j - 1] != '/')
+			path[j++] = path[i];
+	}
+
+	if (j > 1 && path[j - 1] == '/')
+		j--;
+	path[j] = '\0';
+}
+
 static int parse_fsopt_token(char *c, void *private)
 {
 	struct ceph_mount_options *fsopt = private;
@@ -398,73 +418,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)
@@ -472,7 +425,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);
@@ -482,21 +434,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->server_path, fsopt2->server_path);
 	if (ret)
 		return ret;
 
@@ -564,6 +507,8 @@ static int parse_mount_options(struct ceph_mount_options **pfsopt,
 			err = -ENOMEM;
 			goto out;
 		}
+
+		canonicalize_path(fsopt->server_path);
 	} else {
 		dev_name_end = dev_name + strlen(dev_name);
 	}
@@ -990,7 +935,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->server_path ?
+				     fsc->mount_options->server_path + 1 : "";
+
 		err = __ceph_open_session(fsc->client, started);
 		if (err < 0)
 			goto out;
@@ -1002,16 +949,6 @@ 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);
 
 		err = ceph_fs_debugfs_init(fsc);
@@ -1019,7 +956,6 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc)
 			goto out;
 
 		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 8d3eabf06d66..65da12ff5449 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -86,7 +86,7 @@ struct ceph_mount_options {
 
 	char *snapdir_name;   /* default ".snap" */
 	char *mds_namespace;  /* default NULL */
-	char *server_path;    /* default  "/" */
+	char *server_path;    /* default NULL (means "/") */
 	char *fscache_uniq;   /* default NULL */
 };
 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5.4 1/2] ceph: remove the extra slashes in the server path
  2020-04-08 10:58 ` Luis Henriques
                   ` (4 preceding siblings ...)
  (?)
@ 2020-04-08 10:58 ` Luis Henriques
  2020-04-10  9:11   ` Greg Kroah-Hartman
  -1 siblings, 1 reply; 14+ messages in thread
From: Luis Henriques @ 2020-04-08 10:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Sasha Levin
  Cc: Jeff Layton, Ilya Dryomov, ceph-devel, stable, Xiubo Li, Luis Henriques

From: Xiubo Li <xiubli@redhat.com>

commit 4fbc0c711b2464ee1551850b85002faae0b775d5 upstream.

It's possible to pass the mount helper a server path that has more
than one contiguous slash character. For example:

  $ mount -t ceph 192.168.195.165:40176:/// /mnt/cephfs/

In the MDS server side the extra slashes of the server path will be
treated as snap dir, and then we can get the following debug logs:

  ceph:  mount opening path //
  ceph:  open_root_inode opening '//'
  ceph:  fill_trace 0000000059b8a3bc is_dentry 0 is_target 1
  ceph:  alloc_inode 00000000dc4ca00b
  ceph:  get_inode created new inode 00000000dc4ca00b 1.ffffffffffffffff ino 1
  ceph:  get_inode on 1=1.ffffffffffffffff got 00000000dc4ca00b

And then when creating any new file or directory under the mount
point, we can hit the following BUG_ON in ceph_fill_trace():

  BUG_ON(ceph_snap(dir) != dvino.snap);

Have the client ignore the extra slashes in the server path when
mounting. This will also canonicalize the path, so that identical mounts
can be consilidated.

1) "//mydir1///mydir//"
2) "/mydir1/mydir"
3) "/mydir1/mydir/"

Regardless of the internal treatment of these paths, the kernel still
stores the original string including the leading '/' for presentation
to userland.

URL: https://tracker.ceph.com/issues/42771
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Signed-off-by: Luis Henriques <lhenriques@suse.com>
---
 fs/ceph/super.c | 120 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 101 insertions(+), 19 deletions(-)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 62fc7d46032e..54b50452ec1e 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -106,7 +106,6 @@ static int ceph_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return 0;
 }
 
-
 static int ceph_sync_fs(struct super_block *sb, int wait)
 {
 	struct ceph_fs_client *fsc = ceph_sb_to_client(sb);
@@ -430,6 +429,73 @@ 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)
@@ -437,6 +503,7 @@ 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);
@@ -449,9 +516,21 @@ static int compare_mount_options(struct ceph_mount_options *new_fsopt,
 	ret = strcmp_null(fsopt1->mds_namespace, fsopt2->mds_namespace);
 	if (ret)
 		return ret;
-	ret = strcmp_null(fsopt1->server_path, fsopt2->server_path);
+
+	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);
 	if (ret)
 		return ret;
+
 	ret = strcmp_null(fsopt1->fscache_uniq, fsopt2->fscache_uniq);
 	if (ret)
 		return ret;
@@ -507,12 +586,14 @@ static int parse_mount_options(struct ceph_mount_options **pfsopt,
 	 */
 	dev_name_end = strchr(dev_name, '/');
 	if (dev_name_end) {
-		if (strlen(dev_name_end) > 1) {
-			fsopt->server_path = kstrdup(dev_name_end, GFP_KERNEL);
-			if (!fsopt->server_path) {
-				err = -ENOMEM;
-				goto out;
-			}
+		/*
+		 * The server_path will include the whole chars from userland
+		 * including the leading '/'.
+		 */
+		fsopt->server_path = kstrdup(dev_name_end, GFP_KERNEL);
+		if (!fsopt->server_path) {
+			err = -ENOMEM;
+			goto out;
 		}
 	} else {
 		dev_name_end = dev_name + strlen(dev_name);
@@ -842,7 +923,6 @@ static void destroy_caches(void)
 	ceph_fscache_unregister();
 }
 
-
 /*
  * ceph_umount_begin - initiate forced umount.  Tear down down the
  * mount, skipping steps that may hang while waiting for server(s).
@@ -929,9 +1009,6 @@ static struct dentry *open_root_dentry(struct ceph_fs_client *fsc,
 	return root;
 }
 
-
-
-
 /*
  * mount: join the ceph cluster, and open root directory.
  */
@@ -945,7 +1022,7 @@ 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;
+		const char *path, *p;
 		err = __ceph_open_session(fsc->client, started);
 		if (err < 0)
 			goto out;
@@ -957,17 +1034,22 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc)
 				goto out;
 		}
 
-		if (!fsc->mount_options->server_path) {
-			path = "";
-			dout("mount opening path \\t\n");
-		} else {
-			path = fsc->mount_options->server_path + 1;
-			dout("mount opening path %s\n", path);
+		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;

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5.4 2/2] ceph: canonicalize server path in place
  2020-04-08 10:58 ` Luis Henriques
                   ` (5 preceding siblings ...)
  (?)
@ 2020-04-08 10:58 ` Luis Henriques
  -1 siblings, 0 replies; 14+ messages in thread
From: Luis Henriques @ 2020-04-08 10:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Sasha Levin
  Cc: Jeff Layton, Ilya Dryomov, ceph-devel, stable,
	syzbot+98704a51af8e3d9425a9, Luis Henriques

From: Ilya Dryomov <idryomov@gmail.com>

commit b27a939e8376a3f1ed09b9c33ef44d20f18ec3d0 upstream.

syzbot reported that 4fbc0c711b24 ("ceph: remove the extra slashes in
the server path") had caused a regression where an allocation could be
done under a spinlock -- compare_mount_options() is called by sget_fc()
with sb_lock held.

We don't really need the supplied server path, so canonicalize it
in place and compare it directly.  To make this work, the leading
slash is kept around and the logic in ceph_real_mount() to skip it
is restored.  CEPH_MSG_CLIENT_SESSION now reports the same (i.e.
canonicalized) path, with the leading slash of course.

Fixes: 4fbc0c711b24 ("ceph: remove the extra slashes in the server path")
Reported-by: syzbot+98704a51af8e3d9425a9@syzkaller.appspotmail.com
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Luis Henriques <lhenriques@suse.com>
---
 fs/ceph/super.c | 118 +++++++++++-------------------------------------
 fs/ceph/super.h |   2 +-
 2 files changed, 28 insertions(+), 92 deletions(-)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 54b50452ec1e..d40658d5e808 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -214,6 +214,26 @@ static match_table_t fsopt_tokens = {
 	{-1, NULL}
 };
 
+/*
+ * Remove adjacent slashes and then the trailing slash, unless it is
+ * the only remaining character.
+ *
+ * E.g. "//dir1////dir2///" --> "/dir1/dir2", "///" --> "/".
+ */
+static void canonicalize_path(char *path)
+{
+	int i, j = 0;
+
+	for (i = 0; path[i] != '\0'; i++) {
+		if (path[i] != '/' || j < 1 || path[j - 1] != '/')
+			path[j++] = path[i];
+	}
+
+	if (j > 1 && path[j - 1] == '/')
+		j--;
+	path[j] = '\0';
+}
+
 static int parse_fsopt_token(char *c, void *private)
 {
 	struct ceph_mount_options *fsopt = private;
@@ -429,73 +449,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)
@@ -503,7 +456,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);
@@ -513,21 +465,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->server_path, fsopt2->server_path);
 	if (ret)
 		return ret;
 
@@ -595,6 +538,8 @@ static int parse_mount_options(struct ceph_mount_options **pfsopt,
 			err = -ENOMEM;
 			goto out;
 		}
+
+		canonicalize_path(fsopt->server_path);
 	} else {
 		dev_name_end = dev_name + strlen(dev_name);
 	}
@@ -1022,7 +967,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->server_path ?
+				     fsc->mount_options->server_path + 1 : "";
+
 		err = __ceph_open_session(fsc->client, started);
 		if (err < 0)
 			goto out;
@@ -1034,22 +981,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 f98d9247f9cb..bb12c9f3a218 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -92,7 +92,7 @@ struct ceph_mount_options {
 
 	char *snapdir_name;   /* default ".snap" */
 	char *mds_namespace;  /* default NULL */
-	char *server_path;    /* default  "/" */
+	char *server_path;    /* default NULL (means "/") */
 	char *fscache_uniq;   /* default NULL */
 };
 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] stable CephFS backports
  2020-04-08 10:58 ` Luis Henriques
                   ` (6 preceding siblings ...)
  (?)
@ 2020-04-08 13:45 ` Luis Henriques
  2020-04-08 14:50     ` Luis Henriques
  -1 siblings, 1 reply; 14+ messages in thread
From: Luis Henriques @ 2020-04-08 13:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Sasha Levin
  Cc: Jeff Layton, Ilya Dryomov, ceph-devel, stable

On Wed, Apr 08, 2020 at 11:58:38AM +0100, Luis Henriques wrote:
> Hi!
> 
> Please pick the backports for the following upstream commits:

I'm not sure how that happen, but it looks like I've messed up the 2nd
backport.  Please hold on picking these backports for now, I'll need to
figure out how I did that.  I'll send a new version soon.

Cheers,
--
Luís

> 
>   4fbc0c711b24 "ceph: remove the extra slashes in the server path"
>   b27a939e8376 "ceph: canonicalize server path in place"
> 
> They fix an ancient bug that can be reproduced in kernels as old as 4.9 (I
> couldn't reproduced it with 4.4).
> 
> Cheers,
> --
> Luis Henriques
> 
> Ilya Dryomov (1):
>   ceph: canonicalize server path in place
> 
> Xiubo Li (1):
>   ceph: remove the extra slashes in the server path

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] stable CephFS backports
  2020-04-08 13:45 ` [PATCH 0/2] stable CephFS backports Luis Henriques
@ 2020-04-08 14:50     ` Luis Henriques
  0 siblings, 0 replies; 14+ messages in thread
From: Luis Henriques @ 2020-04-08 14:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Sasha Levin
  Cc: Jeff Layton, Ilya Dryomov, ceph-devel, stable

On Wed, Apr 08, 2020 at 02:45:51PM +0100, Luis Henriques wrote:
> On Wed, Apr 08, 2020 at 11:58:38AM +0100, Luis Henriques wrote:
> > Hi!
> > 
> > Please pick the backports for the following upstream commits:
> 
> I'm not sure how that happen, but it looks like I've messed up the 2nd
> backport.  Please hold on picking these backports for now, I'll need to
> figure out how I did that.  I'll send a new version soon.

Ok, I panicked -- backports seem to be fine.  Social isolation isn't new
to me but not seeing other human beings for this long starts making some
(reversible?) damage in my brain :-)

Cheers,
--
Luís

> > 
> >   4fbc0c711b24 "ceph: remove the extra slashes in the server path"
> >   b27a939e8376 "ceph: canonicalize server path in place"
> > 
> > They fix an ancient bug that can be reproduced in kernels as old as 4.9 (I
> > couldn't reproduced it with 4.4).
> > 
> > Cheers,
> > --
> > Luis Henriques
> > 
> > Ilya Dryomov (1):
> >   ceph: canonicalize server path in place
> > 
> > Xiubo Li (1):
> >   ceph: remove the extra slashes in the server path

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] stable CephFS backports
@ 2020-04-08 14:50     ` Luis Henriques
  0 siblings, 0 replies; 14+ messages in thread
From: Luis Henriques @ 2020-04-08 14:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Sasha Levin
  Cc: Jeff Layton, Ilya Dryomov, ceph-devel, stable

On Wed, Apr 08, 2020 at 02:45:51PM +0100, Luis Henriques wrote:
> On Wed, Apr 08, 2020 at 11:58:38AM +0100, Luis Henriques wrote:
> > Hi!
> > 
> > Please pick the backports for the following upstream commits:
> 
> I'm not sure how that happen, but it looks like I've messed up the 2nd
> backport.  Please hold on picking these backports for now, I'll need to
> figure out how I did that.  I'll send a new version soon.

Ok, I panicked -- backports seem to be fine.  Social isolation isn't new
to me but not seeing other human beings for this long starts making some
(reversible?) damage in my brain :-)

Cheers,

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 5.4 1/2] ceph: remove the extra slashes in the server path
  2020-04-08 10:58 ` [PATCH 5.4 1/2] ceph: remove the extra slashes in the server path Luis Henriques
@ 2020-04-10  9:11   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2020-04-10  9:11 UTC (permalink / raw)
  To: Luis Henriques
  Cc: Sasha Levin, Jeff Layton, Ilya Dryomov, ceph-devel, stable, Xiubo Li

On Wed, Apr 08, 2020 at 11:58:43AM +0100, Luis Henriques wrote:
> From: Xiubo Li <xiubli@redhat.com>
> 
> commit 4fbc0c711b2464ee1551850b85002faae0b775d5 upstream.

You forgot about 5.5.y :(


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] stable CephFS backports
  2020-04-08 10:58 ` Luis Henriques
                   ` (7 preceding siblings ...)
  (?)
@ 2020-04-10  9:14 ` Greg Kroah-Hartman
  2020-04-10 10:20   ` Luis Henriques
  -1 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2020-04-10  9:14 UTC (permalink / raw)
  To: Luis Henriques; +Cc: Sasha Levin, Jeff Layton, Ilya Dryomov, ceph-devel, stable

On Wed, Apr 08, 2020 at 11:58:38AM +0100, Luis Henriques wrote:
> Hi!
> 
> Please pick the backports for the following upstream commits:
> 
>   4fbc0c711b24 "ceph: remove the extra slashes in the server path"
>   b27a939e8376 "ceph: canonicalize server path in place"
> 
> They fix an ancient bug that can be reproduced in kernels as old as 4.9 (I
> couldn't reproduced it with 4.4).

All now queued up, including for 5.5.y, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] stable CephFS backports
  2020-04-10  9:14 ` Greg Kroah-Hartman
@ 2020-04-10 10:20   ` Luis Henriques
  0 siblings, 0 replies; 14+ messages in thread
From: Luis Henriques @ 2020-04-10 10:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Sasha Levin, Jeff Layton, Ilya Dryomov, ceph-devel, stable

On Fri, Apr 10, 2020 at 11:14:10AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Apr 08, 2020 at 11:58:38AM +0100, Luis Henriques wrote:
> > Hi!
> > 
> > Please pick the backports for the following upstream commits:
> > 
> >   4fbc0c711b24 "ceph: remove the extra slashes in the server path"
> >   b27a939e8376 "ceph: canonicalize server path in place"
> > 
> > They fix an ancient bug that can be reproduced in kernels as old as 4.9 (I
> > couldn't reproduced it with 4.4).
> 
> All now queued up, including for 5.5.y, thanks.

Awesome, thanks.  And sorry, I forgot to mention in the cover-letter that
5.5 was a clean cherry-pick.

Cheers,
--
Luís

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-04-10 10:20 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-08 10:58 [PATCH 0/2] stable CephFS backports Luis Henriques
2020-04-08 10:58 ` Luis Henriques
2020-04-08 10:58 ` [PATCH 4.9 1/2] ceph: remove the extra slashes in the server path Luis Henriques
2020-04-08 10:58 ` [PATCH 4.9 2/2] ceph: canonicalize server path in place Luis Henriques
2020-04-08 10:58 ` [PATCH 4.14,4.19 1/2] ceph: remove the extra slashes in the server path Luis Henriques
2020-04-08 10:58 ` [PATCH 4.14,4.19 2/2] ceph: canonicalize server path in place Luis Henriques
2020-04-08 10:58 ` [PATCH 5.4 1/2] ceph: remove the extra slashes in the server path Luis Henriques
2020-04-10  9:11   ` Greg Kroah-Hartman
2020-04-08 10:58 ` [PATCH 5.4 2/2] ceph: canonicalize server path in place Luis Henriques
2020-04-08 13:45 ` [PATCH 0/2] stable CephFS backports Luis Henriques
2020-04-08 14:50   ` Luis Henriques
2020-04-08 14:50     ` Luis Henriques
2020-04-10  9:14 ` Greg Kroah-Hartman
2020-04-10 10:20   ` Luis Henriques

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.