Re: [PATCH RESEND 10/11] cifs: Add support for parsing WSL symlinks in version 1 format

Pali Rohár <[email protected]>
Newsgroups org.kernel.vger.linux-cifs,org.kernel.vger.linux-kernel
Message-ID <20260706205006.a4oczzxh4ythlty3@pali>
On Monday 06 July 2026 20:48:18 Pali Rohár wrote:
> -	/* MS-FSCC 2.1.2.7 defines layout of the Target field only for Version 2. */
>  	u32 version = le32_to_cpu(buf->Version);
> +	switch (version) {
> +	case 1:
> +		/*
> +		 * Layout version 1 stores the symlink target in the data section of
> +		 * the file encoded in UTF-8 without trailing null-term byte.
> +		 */
>  
> -	if (version != 2) {
> +		oparms = CIFS_OPARMS(cifs_sb, tcon, full_path, FILE_READ_DATA,
> +				     FILE_OPEN, CREATE_NOT_DIR | OPEN_REPARSE_POINT,
> +				     ACL_NO_MODE);
> +		oparms.fid = &fid;
> +		oplock = tcon->ses->server->oplocks ? REQ_OPLOCK : 0;
> +		rc = tcon->ses->server->ops->open(xid, &oparms, &oplock, NULL);
> +		if (rc)
> +			goto out;
> +
> +		free_symname_utf8 = true;
> +		symname_utf8_len = le64_to_cpu(data->fi.EndOfFile);
> +		symname_utf8 = kmalloc(symname_utf8_len, GFP_KERNEL);
> +		if (!symname_utf8) {
> +			rc = -ENOMEM;
> +			goto out;
> +		}

Sashiko AI wrote:
https://sashiko.dev/#/patchset/20260706184819.22124-1-pali%40kernel.org

  Does this leak the server-side file handle and oplock?
  If this kmalloc() fails, the code jumps to the out label without calling
  tcon->ses->server->ops->close(xid, tcon, &fid).

This is really a problem. I would suggest to move the kmalloc() block
above the tcon->ses->server->ops->open(...) block which should address
this issue. E.g.:

diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index 3418e4b66022..3fe57d166776 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -1133,6 +1133,14 @@ static int parse_reparse_wsl_symlink(struct reparse_wsl_symlink_data_buffer *buf
 		 * the file encoded in UTF-8 without trailing null-term byte.
 		 */

+		free_symname_utf8 = true;
+		symname_utf8_len = le64_to_cpu(data->fi.EndOfFile);
+		symname_utf8 = kmalloc(symname_utf8_len, GFP_KERNEL);
+		if (!symname_utf8) {
+			rc = -ENOMEM;
+			goto out;
+		}
+
 		oparms = CIFS_OPARMS(cifs_sb, tcon, full_path, FILE_READ_DATA,
 				     FILE_OPEN, CREATE_NOT_DIR | OPEN_REPARSE_POINT,
 				     ACL_NO_MODE);
@@ -1142,14 +1150,6 @@ static int parse_reparse_wsl_symlink(struct reparse_wsl_symlink_data_buffer *buf
 		if (rc)
 			goto out;

-		free_symname_utf8 = true;
-		symname_utf8_len = le64_to_cpu(data->fi.EndOfFile);
-		symname_utf8 = kmalloc(symname_utf8_len, GFP_KERNEL);
-		if (!symname_utf8) {
-			rc = -ENOMEM;
-			goto out;
-		}
-
 		buf_type = CIFS_NO_BUFFER;
 		io_parms = (struct cifs_io_parms) {
 			.netfid = fid.netfid,
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.