All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Pali Rohár" <pali@kernel.org>
To: Steve French <sfrench@samba.org>,
	Paulo Alcantara <pc@manguebit.com>,
	Remy Monsen <monsen@monsen.cc>
Cc: linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] cifs: Fix lstat() and AT_SYMLINK_NOFOLLOW to work on broken symlink nodes
Date: Tue, 10 Jun 2025 23:34:04 +0200	[thread overview]
Message-ID: <20250610213404.16288-1-pali@kernel.org> (raw)

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@monsen.cc>
Signed-off-by: Pali Rohár <pali@kernel.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


             reply	other threads:[~2025-06-10 21:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-10 21:34 Pali Rohár [this message]
2025-06-11 14:38 ` [PATCH] cifs: Fix lstat() and AT_SYMLINK_NOFOLLOW to work on broken symlink nodes Steve French
2025-06-20 23:44 ` Paulo Alcantara
2025-06-21 12:21   ` Pali Rohár
2025-06-21 15:38     ` Paulo Alcantara
2025-06-21 16:52       ` Pali Rohár
2025-06-27 21:03         ` Pali Rohár
2025-06-27 22:45           ` Steve French

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250610213404.16288-1-pali@kernel.org \
    --to=pali@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=monsen@monsen.cc \
    --cc=pc@manguebit.com \
    --cc=sfrench@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.