[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <76a3103261ea1be01a83acec8c0db2d4@manguebit.com>
Date: Mon, 30 Sep 2024 12:09:48 -0300
From: Paulo Alcantara <pc@...guebit.com>
To: Pali Rohár <pali@...nel.org>, Steve French
<sfrench@...ba.org>, Ronnie
Sahlberg <ronniesahlberg@...il.com>
Cc: linux-cifs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/7] cifs: Improve creating native symlinks pointing to
directory
Pali Rohár <pali@...nel.org> writes:
> SMB protocol for native symlinks distinguish between symlink to directory
> and symlink to file. These two symlink types cannot be exchanged, which
> means that symlink of file type pointing to directory cannot be resolved at
> all (and vice-versa).
>
> Windows follows this rule for local filesystems (NTFS) and also for SMB.
>
> Linux SMB client currenly creates all native symlinks of file type. Which
> means that Windows (and some other SMB clients) cannot resolve symlinks
> pointing to directory created by Linux SMB client.
>
> As Linux system does not distinguish between directory and file symlinks,
> its API does not provide enough information for Linux SMB client during
> creating of native symlinks.
>
> Add some heuristic into the Linux SMB client for choosing the correct
> symlink type during symlink creation. Check if the symlink target location
> ends with slash, or last path component is dot or dot dot, and check if the
> target location on SMB share exists and is a directory. If at least one
> condition is truth then create a new SMB symlink of directory type.
> Otherwise create it as file type symlink.
>
> This change improves interoperability with Windows systems. Windows systems
> would be able to resolve more SMB symlinks created by Linux SMB client
> which points to existing directory.
>
> Signed-off-by: Pali Rohár <pali@...nel.org>
> ---
> fs/smb/client/reparse.c | 131 ++++++++++++++++++++++++++++++++++++--
> fs/smb/client/smb2inode.c | 3 +-
> fs/smb/client/smb2proto.h | 1 +
> 3 files changed, 130 insertions(+), 5 deletions(-)
>
> diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
> index 507e17244ed3..9390ab801696 100644
> --- a/fs/smb/client/reparse.c
> +++ b/fs/smb/client/reparse.c
> @@ -24,13 +24,16 @@ int smb2_create_reparse_symlink(const unsigned int xid, struct inode *inode,
> struct inode *new;
> struct kvec iov;
> __le16 *path;
> + bool directory = false;
> char *sym, sep = CIFS_DIR_SEP(cifs_sb);
> u16 len, plen;
> int rc = 0;
>
> - sym = kstrdup(symname, GFP_KERNEL);
> + len = strlen(symname)+1;
> + sym = kzalloc(len+1, GFP_KERNEL); /* +1 for possible directory slash */
> if (!sym)
> return -ENOMEM;
> + memcpy(sym, symname, len);
>
> data = (struct cifs_open_info_data) {
> .reparse_point = true,
> @@ -45,6 +48,125 @@ int smb2_create_reparse_symlink(const unsigned int xid, struct inode *inode,
> goto out;
> }
>
> + /*
> + * SMB distinguish between symlink to directory and symlink to file.
> + * They cannot be exchanged (symlink of file type which points to
> + * directory cannot be resolved and vice-versa). First do some simple
> + * check, if the original Linux symlink target ends with slash, or
> + * last path component is dot or dot dot then it is for sure symlink
> + * to the directory.
> + */
> + if (!directory) {
> + const char *basename = kbasename(symname);
> + int basename_len = strlen(basename);
> + if (basename_len == 0 || /* symname ends with slash */
> + (basename_len == 1 && basename[0] == '.') || /* last component is "." */
> + (basename_len == 2 && basename[0] == '.' && basename[1] == '.')) /* last component is ".." */
> + directory = true;
> + }
> +
> + /*
> + * If it was not detected as directory yet and the symlink is relative
> + * then try to resolve the path on the SMB server, check if the path
> + * exists and determinate if it is a directory or not.
> + */
> + if (!directory && symname[0] != '/') {
> + __u32 oplock;
> + struct tcon_link *tlink;
> + struct cifs_tcon *tcon;
> + struct cifs_fid fid;
> + struct cifs_open_parms oparms;
> + char *resolved_path;
> + char *path_sep;
> + int open_rc;
> + int full_path_len = strlen(full_path);
> + int symname_len = strlen(symname);
> +
> + tlink = cifs_sb_tlink(cifs_sb);
> + if (IS_ERR(tlink)) {
> + rc = PTR_ERR(tlink);
> + goto out;
> + }
> +
> + resolved_path = kzalloc(full_path_len + symname_len + 1, GFP_KERNEL);
> + if (!resolved_path) {
> + rc = -ENOMEM;
> + goto out;
> + }
If !@...olved_path, then you will end up leaking @tlink.
> +
> + /*
> + * Compose the resolved SMB symlink path from the SMB full path
> + * and Linux target symlink path.
> + */
> + memcpy(resolved_path, full_path, full_path_len+1);
> + path_sep = strrchr(resolved_path, sep);
> + if (path_sep)
> + path_sep++;
> + else
> + path_sep = resolved_path;
> + memcpy(path_sep, symname, symname_len+1);
> + if (sep == '\\')
> + convert_delimiter(path_sep, sep);
> +
> + tcon = tlink_tcon(tlink);
> +
> + oparms = (struct cifs_open_parms) {
> + .tcon = tcon,
> + .cifs_sb = cifs_sb,
> + .desired_access = FILE_READ_ATTRIBUTES,
> + .disposition = FILE_OPEN,
> + .path = resolved_path,
> + .fid = &fid,
> + };
Please use CIFS_OPARMS().
> +
> + /* Try to open as NOT_FILE */
> + oplock = 0;
> + oparms.create_options = cifs_create_options(cifs_sb, CREATE_NOT_FILE);
> + open_rc = tcon->ses->server->ops->open(xid, &oparms, &oplock, NULL);
> + if (open_rc == 0) {
> + /* Successful open means that the target path is definitely a directory. */
> + directory = true;
> + tcon->ses->server->ops->close(xid, tcon, &fid);
> + } else if (open_rc != -ENOTDIR) {
> + /* Try to open as NOT_DIR */
> + oplock = 0;
> + oparms.create_options = cifs_create_options(cifs_sb, CREATE_NOT_DIR);
> + open_rc = tcon->ses->server->ops->open(xid, &oparms, &oplock, NULL);
> + if (open_rc == 0) {
> + tcon->ses->server->ops->close(xid, tcon, &fid);
> + } else if (open_rc == -EISDIR) {
> + /* -EISDIR means that the target path is definitely a directory. */
> + directory = true;
> + } else {
> + cifs_dbg(FYI,
> + "%s: cannot determinate if the symlink target path '%s' "
> + "is directory or not, creating '%s' as file symlink\n",
> + __func__, symname, full_path);
> + }
> + }
> +
> + kfree(resolved_path);
> + cifs_put_tlink(tlink);
> + }
> +
> + /*
> + * For absolute symlinks it is not possible to determinate
> + * if it should point to directory or file.
> + */
> + if (!directory && symname[0] == '/')
> + cifs_dbg(FYI,
> + "%s: cannot determinate if the symlink target path '%s' "
> + "is directory or not, creating '%s' as file symlink\n",
> + __func__, symname, full_path);
> +
Create a helper with all of this and then call it in
smb2_create_reparse_symlink() to determine whether symlink target is a
directory or file.
Powered by blists - more mailing lists