| 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: <CAH2r5mspnw=BsE-dkzAPC3Q7vMu-pBHYk8k5jcJ1RhwFi=UG5g@mail.gmail.com>
Date: Wed, 11 Jun 2025 09:38:50 -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>, Remy Monsen <monsen@...sen.cc>,
linux-cifs@...r.kernel.org, linux-kernel@...r.kernel.org,
Meetakshi Setiya <meetakshisetiyaoss@...il.com>
Subject: Re: [PATCH] cifs: Fix lstat() and AT_SYMLINK_NOFOLLOW to work on
broken symlink nodes
tentatively merged into cifs-2.6.git for-next pending more testing.
Also added Acked-by from Meetakshi
On Tue, Jun 10, 2025 at 4:34 PM Pali Rohár <pali@...nel.org> wrote:
>
> Currently Linux SMB client returns EIO for lstat() and AT_SYMLINK_NOFOLLOW
> calls on symlink node when the symlink target location is broken or cannot
> be read or parsed.
>
> Fix this problem by relaxing the errors from various locations which parses
> information about symlink file node (UNIX SMB1, native SMB2+, NFS-style,
> WSL-style) and let readlink() syscall to return EIO when the symlink target
> location is not available.
>
> Note that SFU symlinks and MF symlinks are not affected by this issue,
> their parser has already relaxed code.
>
> This change fixes the 'ls -l -a' call on directory which has symlink nodes
> with broken target locations.
>
> Reported-by: Remy Monsen <monsen@...sen.cc>
> Signed-off-by: Pali Rohár <pali@...nel.org>
> ---
> fs/smb/client/cifsfs.c | 3 +-
> fs/smb/client/inode.c | 24 ++++++++++++++++
> fs/smb/client/reparse.c | 61 ++++++++++++++++++++++++++++++++--------
> fs/smb/client/smb2file.c | 10 ++++---
> 4 files changed, 81 insertions(+), 17 deletions(-)
>
> diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
> index a08c42363ffc..f4b923f73dca 100644
> --- a/fs/smb/client/cifsfs.c
> +++ b/fs/smb/client/cifsfs.c
> @@ -1180,7 +1180,8 @@ const char *cifs_get_link(struct dentry *dentry, struct inode *inode,
> strscpy(target_path, CIFS_I(inode)->symlink_target, PATH_MAX);
> } else {
> kfree(target_path);
> - target_path = ERR_PTR(-EOPNOTSUPP);
> + /* If symlink_target is not filled for symlink then it is an IO error. */
> + target_path = ERR_PTR(-EIO);
> }
> spin_unlock(&inode->i_lock);
>
> diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> index b1c6e3986278..762cd194946a 100644
> --- a/fs/smb/client/inode.c
> +++ b/fs/smb/client/inode.c
> @@ -480,6 +480,12 @@ static int cifs_get_unix_fattr(const unsigned char *full_path,
> cifs_sb, full_path,
> &fattr->cf_symlink_target);
> cifs_dbg(FYI, "%s: query_symlink: %d\n", __func__, rc);
> + /*
> + * Convert -EIO to 0. This let lstat() success and
> + * empty data->cf_symlink_target triggers readlink() to fail with -EIO.
> + */
> + if (rc == -EIO)
> + rc = 0;
> }
> return rc;
> }
> @@ -1133,6 +1139,12 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data,
> rc = server->ops->query_symlink(xid, tcon,
> cifs_sb, full_path,
> &data->symlink_target);
> + /*
> + * Convert -EIO to 0. This let lstat() success and
> + * empty data->symlink_target triggers readlink() to fail with -EIO.
> + */
> + if (rc == -EIO)
> + rc = 0;
> }
> if (rc == -EOPNOTSUPP)
> data->reparse.tag = IO_REPARSE_TAG_INTERNAL;
> @@ -1182,6 +1194,18 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data,
> */
> if (rc == -EOPNOTSUPP)
> rc = 0;
> + } else if (data->reparse.tag == IO_REPARSE_TAG_SYMLINK) {
> + /*
> + * data->reparse.tag can be set to IO_REPARSE_TAG_SYMLINK
> + * by STATUS_STOPPED_ON_SYMLINK error code. In this case
> + * we do not have a real reparse point iov buffer so
> + * data->reparse.buf and data->reparse.io.iov.iov_base
> + * are not set. And in the case symlink target location
> + * in the struct smb2_symlink_err_rsp is parsable then we
> + * even do not have data->symlink_target. So set rc to 0
> + * which let lstat() success and readlink() to fail.
> + */
> + rc = 0;
> }
>
> if (data->reparse.tag == IO_REPARSE_TAG_SYMLINK && !rc) {
> diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
> index d1d25f5f72ca..c70affb7b7f7 100644
> --- a/fs/smb/client/reparse.c
> +++ b/fs/smb/client/reparse.c
> @@ -739,7 +739,11 @@ static int parse_reparse_nfs(struct reparse_nfs_data_buffer *buf,
> case NFS_SPECFILE_LNK:
> if (len == 0 || (len % 2)) {
> cifs_dbg(VFS, "srv returned malformed nfs symlink buffer\n");
> - return -EIO;
> + /*
> + * This is an -EIO error. Convert it to 0. This let lstat() success and
> + * empty data->symlink_target triggers readlink() to fail with -EIO.
> + */
> + return 0;
> }
> /*
> * Check that buffer does not contain UTF-16 null codepoint
> @@ -747,7 +751,11 @@ static int parse_reparse_nfs(struct reparse_nfs_data_buffer *buf,
> */
> if (UniStrnlen((wchar_t *)buf->DataBuffer, len/2) != len/2) {
> cifs_dbg(VFS, "srv returned null byte in nfs symlink target location\n");
> - return -EIO;
> + /*
> + * This is an -EIO error. Convert it to 0. This let lstat() success and
> + * empty data->symlink_target triggers readlink() to fail with -EIO.
> + */
> + return 0;
> }
> data->symlink_target = cifs_strndup_from_utf16(buf->DataBuffer,
> len, true,
> @@ -986,6 +994,14 @@ int smb2_parse_native_symlink(char **target, const char *buf, unsigned int len,
> if (rc != 0)
> kfree(linux_target);
> kfree(smb_target);
> +
> + /*
> + * Convert -EIO to 0. This let lstat() success and
> + * empty *target triggers readlink() to fail with -EIO.
> + */
> + if (rc == -EIO)
> + rc = 0;
> +
> return rc;
> }
>
> @@ -1004,7 +1020,11 @@ static int parse_reparse_native_symlink(struct reparse_symlink_data_buffer *sym,
> len = le16_to_cpu(sym->SubstituteNameLength);
> if (offs + 20 > plen || offs + len + 20 > plen) {
> cifs_dbg(VFS, "srv returned malformed symlink buffer\n");
> - return -EIO;
> + /*
> + * This is an -EIO error. Convert it to 0. This let lstat() success and
> + * empty data->symlink_target triggers readlink() to fail with -EIO.
> + */
> + return 0;
> }
>
> return smb2_parse_native_symlink(&data->symlink_target,
> @@ -1024,16 +1044,19 @@ static int parse_reparse_wsl_symlink(struct reparse_wsl_symlink_data_buffer *buf
> int symname_utf8_len;
> __le16 *symname_utf16;
> int symname_utf16_len;
> + int rc = 0;
>
> if (len <= data_offset) {
> cifs_dbg(VFS, "srv returned malformed wsl symlink buffer\n");
> - return -EIO;
> + rc = -EIO;
> + goto out;
> }
>
> /* MS-FSCC 2.1.2.7 defines layout of the Target field only for Version 2. */
> if (le32_to_cpu(buf->Version) != 2) {
> cifs_dbg(VFS, "srv returned unsupported wsl symlink version %u\n", le32_to_cpu(buf->Version));
> - return -EIO;
> + rc = -EIO;
> + goto out;
> }
>
> /* Target for Version 2 is in UTF-8 but without trailing null-term byte */
> @@ -1044,17 +1067,21 @@ static int parse_reparse_wsl_symlink(struct reparse_wsl_symlink_data_buffer *buf
> */
> if (strnlen(buf->Target, symname_utf8_len) != symname_utf8_len) {
> cifs_dbg(VFS, "srv returned null byte in wsl symlink target location\n");
> - return -EIO;
> + rc = -EIO;
> + goto out;
> }
> symname_utf16 = kzalloc(symname_utf8_len * 2, GFP_KERNEL);
> - if (!symname_utf16)
> - return -ENOMEM;
> + if (!symname_utf16) {
> + rc = -ENOMEM;
> + goto out;
> + }
> symname_utf16_len = utf8s_to_utf16s(buf->Target, symname_utf8_len,
> UTF16_LITTLE_ENDIAN,
> (wchar_t *) symname_utf16, symname_utf8_len * 2);
> if (symname_utf16_len < 0) {
> kfree(symname_utf16);
> - return symname_utf16_len;
> + rc = symname_utf16_len;
> + goto out;
> }
> symname_utf16_len *= 2; /* utf8s_to_utf16s() returns number of u16 items, not byte length */
>
> @@ -1062,10 +1089,20 @@ static int parse_reparse_wsl_symlink(struct reparse_wsl_symlink_data_buffer *buf
> symname_utf16_len, true,
> cifs_sb->local_nls);
> kfree(symname_utf16);
> - if (!data->symlink_target)
> - return -ENOMEM;
> + if (!data->symlink_target) {
> + rc = -ENOMEM;
> + goto out;
> + }
>
> - return 0;
> +out:
> + /*
> + * Convert -EIO to 0. This let lstat() success and
> + * empty data->symlink_target triggers readlink() to fail with -EIO.
> + */
> + if (rc == -EIO)
> + rc = 0;
> +
> + return rc;
> }
>
> int parse_reparse_point(struct reparse_data_buffer *buf,
> diff --git a/fs/smb/client/smb2file.c b/fs/smb/client/smb2file.c
> index a7f629238830..9ac359f7be43 100644
> --- a/fs/smb/client/smb2file.c
> +++ b/fs/smb/client/smb2file.c
> @@ -76,11 +76,11 @@ int smb2_fix_symlink_target_type(char **target, bool directory, struct cifs_sb_i
> return 0;
>
> if (!*target)
> - return -EIO;
> + return 0;
>
> len = strlen(*target);
> if (!len)
> - return -EIO;
> + return 0;
>
> /*
> * If this is directory symlink and it does not have trailing slash then
> @@ -103,8 +103,10 @@ int smb2_fix_symlink_target_type(char **target, bool directory, struct cifs_sb_i
> * cannot contain slash character. File name with slash is invalid on
> * both Windows and Linux systems. So return an error for such symlink.
> */
> - if (!directory && (*target)[len-1] == '/')
> - return -EIO;
> + if (!directory && (*target)[len-1] == '/') {
> + kfree(*target);
> + *target = NULL;
> + }
>
> return 0;
> }
> --
> 2.20.1
>
>
--
Thanks,
Steve
Powered by blists - more mailing lists