Re: [PATCH] cifs: avoid dup prefix path in dfs_get_automount_devname()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/16/2023 2:38 PM, Paulo Alcantara wrote:
@server->origin_fullpath already contains the tree name + optional
prefix, so avoid calling __build_path_from_dentry_optional_prefix() as
it might end up duplicating prefix path from @cifs_sb->prepath into
final full path.

Instead, generate DFS full path by simply merging
@server->origin_fullpath with dentry's path.

This fixes the following case

	mount.cifs //root/dfs/dir /mnt/ -o ...
	ls /mnt/link

where cifs_dfs_do_automount() will call smb3_parse_devname() with
@devname set to "//root/dfs/dir/link" instead of
"//root/dfs/dir/dir/link".

Fixes: 7ad54b98fc1f ("cifs: use origin fullpath for automounts")
Signed-off-by: Paulo Alcantara (SUSE) <pc@xxxxxxxxxxxxx>
---
FYI, updated DFS tests to include the above case and avoid regressions.

  fs/cifs/cifs_dfs_ref.c |  2 --
  fs/cifs/dfs.h          | 22 ++++++++++++++++++----
  2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c
index cb40074feb3e..0329a907bdfe 100644
--- a/fs/cifs/cifs_dfs_ref.c
+++ b/fs/cifs/cifs_dfs_ref.c
@@ -171,8 +171,6 @@ static struct vfsmount *cifs_dfs_do_automount(struct path *path)
  		mnt = ERR_CAST(full_path);
  		goto out;
  	}
-
-	convert_delimiter(full_path, '/');
  	cifs_dbg(FYI, "%s: full_path: %s\n", __func__, full_path);
tmp = *cur_ctx;
diff --git a/fs/cifs/dfs.h b/fs/cifs/dfs.h
index 13f26e01f7b9..0b8cbf721fff 100644
--- a/fs/cifs/dfs.h
+++ b/fs/cifs/dfs.h
@@ -34,19 +34,33 @@ static inline int dfs_get_referral(struct cifs_mount_ctx *mnt_ctx, const char *p
  			      cifs_remap(cifs_sb), path, ref, tl);
  }
+/* Return DFS full path out of a dentry set for automount */
  static inline char *dfs_get_automount_devname(struct dentry *dentry, void *page)
  {
  	struct cifs_sb_info *cifs_sb = CIFS_SB(dentry->d_sb);
  	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
  	struct TCP_Server_Info *server = tcon->ses->server;
+	size_t len;
+	char *s;
if (unlikely(!server->origin_fullpath))
  		return ERR_PTR(-EREMOTE);
- return __build_path_from_dentry_optional_prefix(dentry, page,
-							server->origin_fullpath,
-							strlen(server->origin_fullpath),
-							true);
+	s = dentry_path_raw(dentry, page, PATH_MAX);
+	if (IS_ERR(s))
+		return s;
+	/* for root, we want "" */
+	if (!s[1])
+		s++;

The above pointer increment is really hard to understand, given the
comment. So, if the result is a single-character path, presumably "/",
advance the pointer so it becomes a null string? It's not obvious from
this code and comment.

+	len = strlen(server->origin_fullpath);
+	if (s < (char *)page + len)
+		return ERR_PTR(-ENAMETOOLONG);
+
+	s -= len;

This looks doubly dangerous. What prevents the pointer from moving
backwards to ahead of the buffer? Especially in light of the above
root-only adjustment?

+	memcpy(s, server->origin_fullpath, len);
+	convert_delimiter(s, '/');
+	return s;
  }
static inline void dfs_put_root_smb_sessions(struct list_head *head)

Tom.



[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux