[PATCHv2] smb3: allow files to be created with backslash in name

[Paulo Alcantara]

Steve French <smfrench at gmail.com> writes:

> Updated patch (rebased to current for-next-next)
>
> 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 generic/453
>     https://bugzilla.kernel.org/show_bug.cgi?id=210961
>
> Reported-by: Xiaoli Feng <xifeng at redhat.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=210961
> Signed-off-by: Steve French <stfrench at microsoft.com>
>
>
> @Paulo Alcantara  any thoughts if additional changes needed for DFS or
> prefixpaths?

Yes - these changes break DFS mounts with iocharsets different than
utf8.  Consider dfs_cache_canonical_path() where the backslashes will
get remapped in cifs_strndup_to_utf16() and then broken DFS referral
paths will be sent over the wire.

You can simply reproduce this with

        mount.cifs //srv/dfs /mnt -o ...,iocharset=ascii

> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> index 55b3ce944022..e6f4a28275a8 100644
> --- a/fs/smb/client/cifsglob.h
> +++ b/fs/smb/client/cifsglob.h
> @@ -1568,10 +1568,7 @@ CIFS_FILE_SB(struct file *file)
>  
>  static inline char CIFS_DIR_SEP(const struct cifs_sb_info *cifs_sb)
>  {
> -	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIX_PATHS)
> -		return '/';
> -	else
> -		return '\\';
> +	return '/';
>  }

This change breakes readdir(2) under SMB1 mounts (no UNIX extensions)
because CIFSFindFirst() ends up appending "/*" rather than "\\*" to
filename and then fails with STATUS_OBJECT_NAME_INVALID.

You'd need to check all other places where we could also append an UTF16
string with CIFS_DIR_SEP() after getting the path with
cifs_convert_path_to_utf16().

> diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
> index 580a27a3a7e6..6c446b1210ba 100644
> --- a/fs/smb/client/dir.c
> +++ b/fs/smb/client/dir.c
> @@ -160,12 +160,18 @@ check_name(struct dentry *direntry, struct cifs_tcon *tcon)
>  		     le32_to_cpu(tcon->fsAttrInfo.MaxPathNameComponentLength)))
>  		return -ENAMETOOLONG;
>  
> -	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIX_PATHS)) {
> -		for (i = 0; i < direntry->d_name.len; i++) {
> -			if (direntry->d_name.name[i] == '\\') {
> -				cifs_dbg(FYI, "Invalid file name\n");
> -				return -EINVAL;
> -			}
> +	/*
> +	 * SMB3.1.1 POSIX Extensions, CIFS Unix Extensions and SFM mappings
> +	 * allow \ in paths (or in latter case remaps \ to 0xF026)
> +	 */
> +	if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIX_PATHS) ||
> +	    (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SFM_CHR))

What about

        if (cifs_sb->mnt_cifs_flags & (CIFS_MOUNT_POSIX_PATHS | CIFS_MOUNT_MAP_SFM_CHR))

> diff --git a/fs/smb/client/smb2misc.c b/fs/smb/client/smb2misc.c
> index e20b4354e703..0edc888ec3f8 100644
> --- a/fs/smb/client/smb2misc.c
> +++ b/fs/smb/client/smb2misc.c
> @@ -469,13 +469,17 @@ cifs_convert_path_to_utf16(const char *from, struct cifs_sb_info *cifs_sb)
>  	if (from[0] == '\\')
>  		start_of_path = from + 1;
>  
> -	/* SMB311 POSIX extensions paths do not include leading slash */
> -	else if (cifs_sb_master_tlink(cifs_sb) &&
> -		 cifs_sb_master_tcon(cifs_sb)->posix_extensions &&
> -		 (from[0] == '/')) {
> -		start_of_path = from + 1;
> -	} else
> -		start_of_path = from;
> +	start_of_path = from;
> +	/*
> +	 * Only old CIFS Unix extensions paths include leading slash
> +	 * Need to skip if for SMB3.1.1 POSIX Extensions and SMB1/2/3
> +	 */
> +	if (from[0] == '/') {
> +		if (((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIX_PATHS) == false) ||

I guess you meant "== 0".  Also, no need for extra parentheses.