| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAH2r5muBtrnwm4YXm_DuncR7bK68xwY3OP2GwV07-LEm-BPWTw@mail.gmail.com>
Date: Sun, 29 Sep 2024 16:54:20 -0500
From: Steve French <smfrench@...il.com>
To: Pali Rohár <pali@...nel.org>
Cc: Steve French <sfrench@...ba.org>, Paulo Alcantara <pc@...guebit.com>,
Ronnie Sahlberg <ronniesahlberg@...il.com>, linux-cifs@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/7] cifs: Improve creating native symlinks pointing to directory
Here is a version of the same patch without the function rename (so
presumably easier to backport) and also that fixes to minor checkpatch
warnings (and merged this and also patches 3, 4 and 6 into
cifs-2.6.git for-next pending additional review and tesitng):
On Sun, Sep 29, 2024 at 1:51 PM Pali Rohár <pali@...nel.org> wrote:
>
> 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;
> + }
> +
> + /*
> + * 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,
> + };
> +
> + /* 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);
> +
> + /* Ensure that directory symlink target in inode would have trailing slash */
> + len = strlen(data.symlink_target);
> + if (directory && data.symlink_target[len-1] != '/') {
> + /* symlink_target has already preallocated one byte more */
> + data.symlink_target[len] = '/';
> + data.symlink_target[len+1] = '\0';
> + }
> +
> plen = 2 * UniStrnlen((wchar_t *)path, PATH_MAX);
> len = sizeof(*buf) + plen * 2;
> buf = kzalloc(len, GFP_KERNEL);
> @@ -69,7 +191,8 @@ int smb2_create_reparse_symlink(const unsigned int xid, struct inode *inode,
> iov.iov_base = buf;
> iov.iov_len = len;
> new = smb2_create_reparse_inode(&data, inode->i_sb, xid,
> - tcon, full_path, &iov, NULL);
> + tcon, full_path, directory,
> + &iov, NULL);
> if (!IS_ERR(new))
> d_instantiate(dentry, new);
> else
> @@ -137,7 +260,7 @@ static int mknod_nfs(unsigned int xid, struct inode *inode,
> };
>
> new = smb2_create_reparse_inode(&data, inode->i_sb, xid,
> - tcon, full_path, &iov, NULL);
> + tcon, full_path, false, &iov, NULL);
> if (!IS_ERR(new))
> d_instantiate(dentry, new);
> else
> @@ -283,7 +406,7 @@ static int mknod_wsl(unsigned int xid, struct inode *inode,
> data.wsl.eas_len = len;
>
> new = smb2_create_reparse_inode(&data, inode->i_sb,
> - xid, tcon, full_path,
> + xid, tcon, full_path, false,
> &reparse_iov, &xattr_iov);
> if (!IS_ERR(new))
> d_instantiate(dentry, new);
> diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> index 0fc73035d6dc..fffb9df4faeb 100644
> --- a/fs/smb/client/smb2inode.c
> +++ b/fs/smb/client/smb2inode.c
> @@ -1198,6 +1198,7 @@ struct inode *smb2_create_reparse_inode(struct cifs_open_info_data *data,
> const unsigned int xid,
> struct cifs_tcon *tcon,
> const char *full_path,
> + bool directory,
> struct kvec *reparse_iov,
> struct kvec *xattr_iov)
> {
> @@ -1217,7 +1218,7 @@ struct inode *smb2_create_reparse_inode(struct cifs_open_info_data *data,
> FILE_READ_ATTRIBUTES |
> FILE_WRITE_ATTRIBUTES,
> FILE_CREATE,
> - CREATE_NOT_DIR | OPEN_REPARSE_POINT,
> + (directory ? CREATE_NOT_FILE : CREATE_NOT_DIR) | OPEN_REPARSE_POINT,
> ACL_NO_MODE);
> if (xattr_iov)
> oparms.ea_cctx = xattr_iov;
> diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h
> index 4ac30d29d5a1..fd092f2bb8c5 100644
> --- a/fs/smb/client/smb2proto.h
> +++ b/fs/smb/client/smb2proto.h
> @@ -61,6 +61,7 @@ struct inode *smb2_create_reparse_inode(struct cifs_open_info_data *data,
> const unsigned int xid,
> struct cifs_tcon *tcon,
> const char *full_path,
> + bool directory,
> struct kvec *reparse_iov,
> struct kvec *xattr_iov);
> int smb2_query_reparse_point(const unsigned int xid,
> --
> 2.20.1
>
>
--
Thanks,
Steve
View attachment "0001-cifs-Improve-creating-native-symlinks-pointing-to-di.patch" of type "text/x-patch" (9008 bytes)
Powered by blists - more mailing lists