[PATCH][SMB3] skip extra NULL byte in filenames

[PATCH][SMB3] skip extra NULL byte in filenames

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 787 bytes --]

Any comments on Paulo's recent patch below?

    Since commit:
     cifs: alloc_path_with_tree_prefix: do not append sep. if the path is empty
    alloc_path_with_tree_prefix() function was no longer including the
    trailing separator when @path is empty, although @out_len was still
    assuming a path separator thus adding an extra byte to the final
    filename.

    This has caused mount issues in some Synology servers due to the extra
    NULL byte in filenames when sending SMB2_CREATE requests with
    SMB2_FLAGS_DFS_OPERATIONS set.

    Fix this by checking if @path is not empty and then add extra byte for
    separator.  Also, do not include any trailing NULL bytes in filename
    as MS-SMB2 requires it to be 8-byte aligned and not NULL terminated.

--
Thanks,

Steve

[-- Attachment #2: git.cjr.nz.patch --]
[-- Type: application/x-patch, Size: 2135 bytes --]

[-- Attachment #3: 0001-cifs-skip-extra-NULL-byte-in-filenames.patch --]
[-- Type: text/x-patch, Size: 2182 bytes --]

From 93bbb2c27d8543abb03254a0b5d953c5f3f0b501 Mon Sep 17 00:00:00 2001
From: Paulo Alcantara <pc@cjr.nz>
Date: Fri, 19 Aug 2022 17:00:19 -0300
Subject: [PATCH] cifs: skip extra NULL byte in filenames

Since commit:
 cifs: alloc_path_with_tree_prefix: do not append sep. if the path is empty
alloc_path_with_tree_prefix() function was no longer including the
trailing separator when @path is empty, although @out_len was still
assuming a path separator thus adding an extra byte to the final
filename.

This has caused mount issues in some Synology servers due to the extra
NULL byte in filenames when sending SMB2_CREATE requests with
SMB2_FLAGS_DFS_OPERATIONS set.

Fix this by checking if @path is not empty and then add extra byte for
separator.  Also, do not include any trailing NULL bytes in filename
as MS-SMB2 requires it to be 8-byte aligned and not NULL terminated.

Fixes: 7eacba3b00a3 ("cifs: alloc_path_with_tree_prefix: do not append sep. if the path is empty")
Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/smb2pdu.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 91cfc5b47ac7..128e44e57528 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -2572,19 +2572,15 @@ alloc_path_with_tree_prefix(__le16 **out_path, int *out_size, int *out_len,
 
 	path_len = UniStrnlen((wchar_t *)path, PATH_MAX);
 
-	/*
-	 * make room for one path separator between the treename and
-	 * path
-	 */
-	*out_len = treename_len + 1 + path_len;
+	/* make room for one path separator only if @path isn't empty */
+	*out_len = treename_len + (path[0] ? 1 : 0) + path_len;
 
 	/*
-	 * final path needs to be null-terminated UTF16 with a
-	 * size aligned to 8
+	 * final path needs to be 8-byte aligned as specified in
+	 * MS-SMB2 2.2.13 SMB2 CREATE Request.
 	 */
-
-	*out_size = roundup((*out_len+1)*2, 8);
-	*out_path = kzalloc(*out_size, GFP_KERNEL);
+	*out_size = roundup(*out_len * sizeof(__le16), 8);
+	*out_path = kzalloc(*out_size + sizeof(__le16) /* null */, GFP_KERNEL);
 	if (!*out_path)
 		return -ENOMEM;
 
-- 
2.34.1

Re: [PATCH][SMB3] skip extra NULL byte in filenames

[Enzo Matsumiya]

On 08/23, Steve French wrote:
>Any comments on Paulo's recent patch below?
>
>    Since commit:
>     cifs: alloc_path_with_tree_prefix: do not append sep. if the path is empty
>    alloc_path_with_tree_prefix() function was no longer including the
>    trailing separator when @path is empty, although @out_len was still
>    assuming a path separator thus adding an extra byte to the final
>    filename.
>
>    This has caused mount issues in some Synology servers due to the extra
>    NULL byte in filenames when sending SMB2_CREATE requests with
>    SMB2_FLAGS_DFS_OPERATIONS set.
>
>    Fix this by checking if @path is not empty and then add extra byte for
>    separator.  Also, do not include any trailing NULL bytes in filename
>    as MS-SMB2 requires it to be 8-byte aligned and not NULL terminated.

Reviewed-by: Enzo Matsumiya <ematsumiya@suse.de>


Cheers,

Enzo

>--
>Thanks,
>
>Steve