[PATCH][SMB3] allow files to be created with backslash in file name

* [PATCH][SMB3] allow files to be created with backslash in file name
@ 2021-01-01  3:35 Steve French
  2021-01-01  6:00 ` Jeremy Allison
  0 siblings, 1 reply; 12+ messages in thread
From: Steve French @ 2021-01-01  3:35 UTC (permalink / raw)
  To: Xiaoli Feng, CIFS; +Cc: samba-technical

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

Backslash is reserved in Windows (and SMB2/SMB3 by default) but
allowed in POSIX so must be remapped when POSIX extensions are
not enabled.

The default mapping for SMB3 mounts ("SFM") allows mapping backslash
(ie 0x5C in UTF8) to 0xF026 in UCS-2 (using the Unicode remapping
range reserved for these characters), but this was not mapped by
cifs.ko (unlike asterisk, greater than, question mark etc).  This patch
fixes that to allow creating files and directories with backslash
in the file or directory name.

Before this patch:
   touch "/mnt2/filewith\slash"
would return
   touch: setting times of '/mnt2/filewith\slash': Invalid argument

With the patch touch and mkdir with the backslash in the name works.

This problem was found while debugging xfstest 453 - see
    https://bugzilla.kernel.org/show_bug.cgi?id=210961

This patch may be even more important to Samba, as alternative ways of
storing these files can create more problems. Interestingly Samba
server reports local files with backslashes in them over the wire
without remapping, even though these are illegal in SMB3 which would
cause confusion on the client(s).  Has anyone tried Windows mounting
to Samba and viewing files with locally created Linux files that
include these reserved characters (presumably Windows clients won't
like it either)?

This patch does allow creating/viewing files with remotely with '\' in
the file name from Linux (cifs.ko) but does not completely fix xfstest
453 kernel (more investigation of this test is needed).  Test 453
creates filenames with 'horrifying'
(using their term) sequences of arbitrary bytes in file names.

Reported-by: Xiaoli Feng <xifeng@redhat.com>

-- 
Thanks,

Steve

[-- Attachment #2: 0001-smb3-allow-files-to-be-created-with-backslash-in-fil.patch --]
[-- Type: text/x-patch, Size: 2856 bytes --]

From 03596841d4119840548676a9b6d916580baa618c Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Thu, 31 Dec 2020 21:12:19 -0600
Subject: [PATCH] smb3: allow files to be created with backslash in file name

Backslash is reserved in Windows (and SMB2/SMB3 by default) but
allowed in POSIX so must be remapped when POSIX extensions are
not enabled.

The default mapping for SMB3 mounts ("SFM") allows mapping backslash
(ie 0x5C in UTF8) to 0xF026 in UCS-2 (using the Unicode remapping
range reserved for these characters), but this was not mapped by
cifs.ko (unlike asterisk, greater than, question mark etc).  This patch
fixes that to allow creating files and directories with backslash
in the file or directory name.

Before this patch:
   touch "/mnt2/filewith\slash"
would return
   touch: setting times of '/mnt2/filewith\slash': Invalid argument

With the patch touch and mkdir with the backslash in the name works.

This problem was found while debugging
    https://bugzilla.kernel.org/show_bug.cgi?id=210961

Reported-by: Xiaoli Feng <xifeng@redhat.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/cifs_unicode.c |  6 ++++++
 fs/cifs/dir.c          | 10 ++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c
index 9bd03a231032..a65310c87db4 100644
--- a/fs/cifs/cifs_unicode.c
+++ b/fs/cifs/cifs_unicode.c
@@ -98,6 +98,9 @@ convert_sfm_char(const __u16 src_char, char *target)
 	case SFM_PERIOD:
 		*target = '.';
 		break;
+	case SFM_SLASH:
+		*target = '\\';
+		break;
 	default:
 		return false;
 	}
@@ -431,6 +434,9 @@ static __le16 convert_to_sfm_char(char src_char, bool end_of_string)
 	case '|':
 		dest_char = cpu_to_le16(SFM_PIPE);
 		break;
+	case '\\':
+		dest_char = cpu_to_le16(SFM_SLASH);
+		break;
 	case '.':
 		if (end_of_string)
 			dest_char = cpu_to_le16(SFM_PERIOD);
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 68900f1629bf..62cc589e33cd 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -208,8 +208,14 @@ check_name(struct dentry *direntry, struct cifs_tcon *tcon)
 		     direntry->d_name.len >
 		     le32_to_cpu(tcon->fsAttrInfo.MaxPathNameComponentLength)))
 		return -ENAMETOOLONG;
-
-	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIX_PATHS)) {
+	/*
+	 * If POSIX extensions negotiated or if mapping reserved characters
+	 * (via SFM, the default on most mounts currently, then '\' is
+	 * remapped on the wire into the Unicode reserved range, 0xF026) then
+	 * do not need to reject get/set requests with backslash in path
+	 */
+	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIX_PATHS) &&
+	    !(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SFM_CHR)) {
 		for (i = 0; i < direntry->d_name.len; i++) {
 			if (direntry->d_name.name[i] == '\\') {
 				cifs_dbg(FYI, "Invalid file name\n");
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread