public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] cifs: Fixes for SMB1 non-UNICODE 8-bit mode
@ 2024-10-28 11:03 Pali Rohár
  2024-10-28 11:03 ` [PATCH 1/5] cifs: Add new mount option -o nounicode to disable SMB1 UNICODE mode Pali Rohár
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Pali Rohár @ 2024-10-28 11:03 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

SMB1 protocol supports non-UNICODE (8-bit OEM character set) and
UNICODE (UTF-16) modes. Linux SMB1 client implements both of them but
there are few bugs in processing non-UNICODE mode.

This patch series add a new mount option -o nounicode to disable UNICODE
mode and force usage of non-UNICODE (8-bit OEM character set) mode. This
allows to test non-UNICODE code path against modern/recent SMB servers
which implements and prefer UNICODE mode.

And this patch series fixes SMB1 session setup and reading symlinks when
UNICODE mode is not active.

Tested against Windows Server 2022 SMB1 server and older Samba SMB1
server.

Pali Rohár (5):
  cifs: Add new mount option -o nounicode to disable SMB1 UNICODE mode
  cifs: Fix encoding of SMB1 Session Setup Kerberos Request in
    non-UNICODE mode
  cifs: Add support for SMB1 Session Setup NTLMSSP Request in
    non-UNICODE mode
  cifs: Fix parsing reparse point with native symlink in SMB1
    non-UNICODE session
  cifs: Remove unicode parameter from parse_reparse_point() function

 fs/smb/client/cifsfs.c     |  4 ++
 fs/smb/client/cifsglob.h   |  2 +
 fs/smb/client/cifsproto.h  |  2 +-
 fs/smb/client/cifssmb.c    |  5 ++-
 fs/smb/client/connect.c    | 32 +++++++++++++--
 fs/smb/client/fs_context.c | 11 ++++++
 fs/smb/client/fs_context.h |  2 +
 fs/smb/client/reparse.c    | 25 ++++++------
 fs/smb/client/sess.c       | 81 ++++++++++++++++++++++++--------------
 fs/smb/client/smb1ops.c    |  4 +-
 fs/smb/client/smb2file.c   |  1 -
 fs/smb/client/smb2proto.h  |  2 +-
 12 files changed, 118 insertions(+), 53 deletions(-)

-- 
2.20.1


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

* [PATCH 1/5] cifs: Add new mount option -o nounicode to disable SMB1 UNICODE mode
  2024-10-28 11:03 [PATCH 0/5] cifs: Fixes for SMB1 non-UNICODE 8-bit mode Pali Rohár
@ 2024-10-28 11:03 ` Pali Rohár
  2024-11-15 13:26   ` Björn JACKE
  2024-10-28 11:03 ` [PATCH 2/5] cifs: Fix encoding of SMB1 Session Setup Kerberos Request in non-UNICODE mode Pali Rohár
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Pali Rohár @ 2024-10-28 11:03 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

SMB1 protocol supports non-UNICODE (8-bit OEM character set) and
UNICODE (UTF-16) modes.

Linux SMB1 client implements both of them but currently does not allow to
choose non-UNICODE mode when SMB1 server announce UNICODE mode support.

This change adds a new mount option -o nounicode to disable UNICODE mode
and force usage of non-UNICODE (8-bit OEM character set) mode.

This allows to test non-UNICODE implementation of Linux SMB1 client against
any SMB1 server, including modern and recent Windows SMB1 server.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifsfs.c     |  4 ++++
 fs/smb/client/cifsglob.h   |  2 ++
 fs/smb/client/cifssmb.c    |  5 ++++-
 fs/smb/client/connect.c    | 32 +++++++++++++++++++++++++++++---
 fs/smb/client/fs_context.c | 11 +++++++++++
 fs/smb/client/fs_context.h |  2 ++
 fs/smb/client/sess.c       |  1 +
 fs/smb/client/smb1ops.c    |  1 +
 8 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 1decf91d3f61..447ed7831f2c 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -611,6 +611,10 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
 					   cifs_sb->ctx->dir_mode);
 	if (cifs_sb->ctx->iocharset)
 		seq_printf(s, ",iocharset=%s", cifs_sb->ctx->iocharset);
+	if (tcon->ses->unicode == 0)
+		seq_puts(s, ",nounicode");
+	else if (tcon->ses->unicode == 1)
+		seq_puts(s, ",unicode");
 	if (tcon->seal)
 		seq_puts(s, ",seal");
 	else if (tcon->ses->server->ignore_signature)
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index dcee43889358..2f77b558abe8 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -651,6 +651,7 @@ struct smb_version_values {
 	unsigned int	cap_unix;
 	unsigned int	cap_nt_find;
 	unsigned int	cap_large_files;
+	unsigned int	cap_unicode;
 	__u16		signing_enabled;
 	__u16		signing_required;
 	size_t		create_lease_size;
@@ -1124,6 +1125,7 @@ struct cifs_ses {
 	bool sign;		/* is signing required? */
 	bool domainAuto:1;
 	bool expired_pwd;  /* track if access denied or expired pwd so can know if need to update */
+	int unicode;
 	unsigned int flags;
 	__u16 session_flags;
 	__u8 smb3signingkey[SMB3_SIGN_KEY_SIZE];
diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index c6f15dbe860a..6218b59b9da7 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -423,7 +423,10 @@ CIFSSMBNegotiate(const unsigned int xid,
 		return rc;
 
 	pSMB->hdr.Mid = get_next_mid(server);
-	pSMB->hdr.Flags2 |= (SMBFLG2_UNICODE | SMBFLG2_ERR_STATUS);
+	pSMB->hdr.Flags2 |= SMBFLG2_ERR_STATUS;
+
+	if (ses->unicode != 0)
+		pSMB->hdr.Flags2 |= SMBFLG2_UNICODE;
 
 	if (should_set_ext_sec_flag(ses->sectype)) {
 		cifs_dbg(FYI, "Requesting extended security\n");
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 612816ec71f5..3d9d736b6e58 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -2338,6 +2338,7 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
 	ses->cred_uid = ctx->cred_uid;
 	ses->linux_uid = ctx->linux_uid;
 
+	ses->unicode = ctx->unicode;
 	ses->sectype = ctx->sectype;
 	ses->sign = ctx->sign;
 	ses->local_nls = load_nls(ctx->local_nls->charset);
@@ -3928,7 +3929,7 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
 		   struct TCP_Server_Info *server,
 		   struct nls_table *nls_info)
 {
-	int rc = -ENOSYS;
+	int rc = 0;
 	struct TCP_Server_Info *pserver = SERVER_IS_CHAN(server) ? server->primary_server : server;
 	struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)&pserver->dstaddr;
 	struct sockaddr_in *addr = (struct sockaddr_in *)&pserver->dstaddr;
@@ -3980,6 +3981,26 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
 		if (!linuxExtEnabled)
 			ses->capabilities &= (~server->vals->cap_unix);
 
+		/*
+		 * Check if the server supports specified encoding mode.
+		 * Zero value in vals->cap_unicode indidcates that chosen
+		 * protocol dialect does not support non-UNICODE mode.
+		 */
+		if (ses->unicode == 1 && server->vals->cap_unicode != 0 &&
+		    !(server->capabilities & server->vals->cap_unicode)) {
+			cifs_dbg(VFS, "Server does not support mounting in UNICODE mode\n");
+			rc = -EOPNOTSUPP;
+		} else if (ses->unicode == 0 && server->vals->cap_unicode == 0) {
+			cifs_dbg(VFS, "Server does not support mounting in non-UNICODE mode\n");
+			rc = -EOPNOTSUPP;
+		} else if (ses->unicode == 0) {
+			/*
+			 * When UNICODE mode was explicitly disabled then
+			 * do not announce client UNICODE capability.
+			 */
+			ses->capabilities &= (~server->vals->cap_unicode);
+		}
+
 		if (ses->auth_key.response) {
 			cifs_dbg(FYI, "Free previous auth_key.response = %p\n",
 				 ses->auth_key.response);
@@ -3992,8 +4013,12 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
 	cifs_dbg(FYI, "Security Mode: 0x%x Capabilities: 0x%x TimeAdjust: %d\n",
 		 server->sec_mode, server->capabilities, server->timeAdj);
 
-	if (server->ops->sess_setup)
-		rc = server->ops->sess_setup(xid, ses, server, nls_info);
+	if (!rc) {
+		if (server->ops->sess_setup)
+			rc = server->ops->sess_setup(xid, ses, server, nls_info);
+		else
+			rc = -ENOSYS;
+	}
 
 	if (rc) {
 		cifs_server_dbg(VFS, "Send error in SessSetup = %d\n", rc);
@@ -4063,6 +4088,7 @@ cifs_construct_tcon(struct cifs_sb_info *cifs_sb, kuid_t fsuid)
 	ctx->seal = master_tcon->seal;
 	ctx->witness = master_tcon->use_witness;
 	ctx->dfs_root_ses = master_tcon->ses->dfs_root_ses;
+	ctx->unicode = master_tcon->ses->unicode;
 
 	rc = cifs_set_vol_auth(ctx, master_tcon->ses);
 	if (rc) {
diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index 7dd46cfc9fb3..d970b66e529c 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -127,6 +127,7 @@ const struct fs_parameter_spec smb3_fs_parameters[] = {
 	fsparam_flag("rootfs", Opt_rootfs),
 	fsparam_flag("compress", Opt_compress),
 	fsparam_flag("witness", Opt_witness),
+	fsparam_flag_no("unicode", Opt_unicode),
 
 	/* Mount options which take uid or gid */
 	fsparam_uid("backupuid", Opt_backupuid),
@@ -930,6 +931,10 @@ static int smb3_verify_reconfigure_ctx(struct fs_context *fc,
 		cifs_errorf(fc, "can not change iocharset during remount\n");
 		return -EINVAL;
 	}
+	if (new_ctx->unicode != old_ctx->unicode) {
+		cifs_errorf(fc, "can not change unicode during remount\n");
+		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -1520,6 +1525,10 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
 		ctx->witness = true;
 		pr_warn_once("Witness protocol support is experimental\n");
 		break;
+	case Opt_unicode:
+		ctx->unicode = !result.negated;
+		cifs_dbg(FYI, "unicode set to %d\n", ctx->unicode);
+		break;
 	case Opt_rootfs:
 #ifndef CONFIG_CIFS_ROOT
 		cifs_dbg(VFS, "rootfs support requires CONFIG_CIFS_ROOT config option\n");
@@ -1816,6 +1825,8 @@ int smb3_init_fs_context(struct fs_context *fc)
 	ctx->symlink_type = CIFS_SYMLINK_TYPE_DEFAULT;
 	ctx->nonativesocket = 0;
 
+	ctx->unicode = -1; /* autodetect, but prefer UNICODE mode */
+
 /*
  *	short int override_uid = -1;
  *	short int override_gid = -1;
diff --git a/fs/smb/client/fs_context.h b/fs/smb/client/fs_context.h
index 18d39d457145..1514e05e6629 100644
--- a/fs/smb/client/fs_context.h
+++ b/fs/smb/client/fs_context.h
@@ -127,6 +127,7 @@ enum cifs_param {
 	Opt_multichannel,
 	Opt_compress,
 	Opt_witness,
+	Opt_unicode,
 
 	/* Mount options which take numeric value */
 	Opt_backupuid,
@@ -296,6 +297,7 @@ struct smb3_fs_context {
 	bool compress; /* enable SMB2 messages (READ/WRITE) de/compression */
 	bool rootfs:1; /* if it's a SMB root file system */
 	bool witness:1; /* use witness protocol */
+	int unicode;
 	char *leaf_fullpath;
 	struct cifs_ses *dfs_root_ses;
 	bool dfs_automount:1; /* set for dfs automount only */
diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
index c88e9657f47a..6a92db287843 100644
--- a/fs/smb/client/sess.c
+++ b/fs/smb/client/sess.c
@@ -529,6 +529,7 @@ cifs_ses_add_channel(struct cifs_ses *ses,
 	ctx->password = ses->password;
 	ctx->sectype = ses->sectype;
 	ctx->sign = ses->sign;
+	ctx->unicode = ses->unicode;
 
 	/* UNC and paths */
 	/* XXX: Use ses->server->hostname? */
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index abca214f923c..b0fb4a5c586d 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -1189,6 +1189,7 @@ struct smb_version_values smb1_values = {
 	.cap_unix = CAP_UNIX,
 	.cap_nt_find = CAP_NT_SMBS | CAP_NT_FIND,
 	.cap_large_files = CAP_LARGE_FILES,
+	.cap_unicode = CAP_UNICODE,
 	.signing_enabled = SECMODE_SIGN_ENABLED,
 	.signing_required = SECMODE_SIGN_REQUIRED,
 };
-- 
2.20.1


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

* [PATCH 2/5] cifs: Fix encoding of SMB1 Session Setup Kerberos Request in non-UNICODE mode
  2024-10-28 11:03 [PATCH 0/5] cifs: Fixes for SMB1 non-UNICODE 8-bit mode Pali Rohár
  2024-10-28 11:03 ` [PATCH 1/5] cifs: Add new mount option -o nounicode to disable SMB1 UNICODE mode Pali Rohár
@ 2024-10-28 11:03 ` Pali Rohár
  2024-10-28 11:03 ` [PATCH 3/5] cifs: Add support for SMB1 Session Setup NTLMSSP " Pali Rohár
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Pali Rohár @ 2024-10-28 11:03 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

Like in UNICODE mode, SMB1 Session Setup Kerberos Request contains oslm and
domain strings.

Extract common code into ascii_oslm_strings() and ascii_domain_string()
functions (similar to unicode variants) and use these functions in
non-UNICODE code path in sess_auth_kerberos().

Decision if non-UNICODE or UNICODE mode is used is based on the
SMBFLG2_UNICODE flag in Flags2 packed field, and not based on the
capabilities of server. Fix this check too.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/sess.c | 60 +++++++++++++++++++++++++++++---------------
 1 file changed, 40 insertions(+), 20 deletions(-)

diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
index 6a92db287843..7226f2563b99 100644
--- a/fs/smb/client/sess.c
+++ b/fs/smb/client/sess.c
@@ -701,6 +701,22 @@ unicode_oslm_strings(char **pbcc_area, const struct nls_table *nls_cp)
 	*pbcc_area = bcc_ptr;
 }
 
+static void
+ascii_oslm_strings(char **pbcc_area, const struct nls_table *nls_cp)
+{
+	char *bcc_ptr = *pbcc_area;
+
+	strcpy(bcc_ptr, "Linux version ");
+	bcc_ptr += strlen("Linux version ");
+	strcpy(bcc_ptr, init_utsname()->release);
+	bcc_ptr += strlen(init_utsname()->release) + 1;
+
+	strcpy(bcc_ptr, CIFS_NETWORK_OPSYS);
+	bcc_ptr += strlen(CIFS_NETWORK_OPSYS) + 1;
+
+	*pbcc_area = bcc_ptr;
+}
+
 static void unicode_domain_string(char **pbcc_area, struct cifs_ses *ses,
 				   const struct nls_table *nls_cp)
 {
@@ -725,6 +741,25 @@ static void unicode_domain_string(char **pbcc_area, struct cifs_ses *ses,
 	*pbcc_area = bcc_ptr;
 }
 
+static void ascii_domain_string(char **pbcc_area, struct cifs_ses *ses,
+				const struct nls_table *nls_cp)
+{
+	char *bcc_ptr = *pbcc_area;
+	int len;
+
+	/* copy domain */
+	if (ses->domainName != NULL) {
+		len = strscpy(bcc_ptr, ses->domainName, CIFS_MAX_DOMAINNAME_LEN);
+		if (WARN_ON_ONCE(len < 0))
+			len = CIFS_MAX_DOMAINNAME_LEN - 1;
+		bcc_ptr += len;
+	} /* else we send a null domain name so server will default to its own domain */
+	*bcc_ptr = 0;
+	bcc_ptr++;
+
+	*pbcc_area = bcc_ptr;
+}
+
 static void unicode_ssetup_strings(char **pbcc_area, struct cifs_ses *ses,
 				   const struct nls_table *nls_cp)
 {
@@ -770,25 +805,10 @@ static void ascii_ssetup_strings(char **pbcc_area, struct cifs_ses *ses,
 	*bcc_ptr = 0;
 	bcc_ptr++; /* account for null termination */
 
-	/* copy domain */
-	if (ses->domainName != NULL) {
-		len = strscpy(bcc_ptr, ses->domainName, CIFS_MAX_DOMAINNAME_LEN);
-		if (WARN_ON_ONCE(len < 0))
-			len = CIFS_MAX_DOMAINNAME_LEN - 1;
-		bcc_ptr += len;
-	} /* else we send a null domain name so server will default to its own domain */
-	*bcc_ptr = 0;
-	bcc_ptr++;
-
 	/* BB check for overflow here */
 
-	strcpy(bcc_ptr, "Linux version ");
-	bcc_ptr += strlen("Linux version ");
-	strcpy(bcc_ptr, init_utsname()->release);
-	bcc_ptr += strlen(init_utsname()->release) + 1;
-
-	strcpy(bcc_ptr, CIFS_NETWORK_OPSYS);
-	bcc_ptr += strlen(CIFS_NETWORK_OPSYS) + 1;
+	ascii_domain_string(&bcc_ptr, ses, nls_cp);
+	ascii_oslm_strings(&bcc_ptr, nls_cp);
 
 	*pbcc_area = bcc_ptr;
 }
@@ -1590,7 +1610,7 @@ sess_auth_kerberos(struct sess_data *sess_data)
 	sess_data->iov[1].iov_len = msg->secblob_len;
 	pSMB->req.SecurityBlobLength = cpu_to_le16(sess_data->iov[1].iov_len);
 
-	if (ses->capabilities & CAP_UNICODE) {
+	if (pSMB->req.hdr.Flags2 & SMBFLG2_UNICODE) {
 		/* unicode strings must be word aligned */
 		if (!IS_ALIGNED(sess_data->iov[0].iov_len + sess_data->iov[1].iov_len, 2)) {
 			*bcc_ptr = 0;
@@ -1599,8 +1619,8 @@ sess_auth_kerberos(struct sess_data *sess_data)
 		unicode_oslm_strings(&bcc_ptr, sess_data->nls_cp);
 		unicode_domain_string(&bcc_ptr, ses, sess_data->nls_cp);
 	} else {
-		/* BB: is this right? */
-		ascii_ssetup_strings(&bcc_ptr, ses, sess_data->nls_cp);
+		ascii_oslm_strings(&bcc_ptr, sess_data->nls_cp);
+		ascii_domain_string(&bcc_ptr, ses, sess_data->nls_cp);
 	}
 
 	sess_data->iov[2].iov_len = (long) bcc_ptr -
-- 
2.20.1


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

* [PATCH 3/5] cifs: Add support for SMB1 Session Setup NTLMSSP Request in non-UNICODE mode
  2024-10-28 11:03 [PATCH 0/5] cifs: Fixes for SMB1 non-UNICODE 8-bit mode Pali Rohár
  2024-10-28 11:03 ` [PATCH 1/5] cifs: Add new mount option -o nounicode to disable SMB1 UNICODE mode Pali Rohár
  2024-10-28 11:03 ` [PATCH 2/5] cifs: Fix encoding of SMB1 Session Setup Kerberos Request in non-UNICODE mode Pali Rohár
@ 2024-10-28 11:03 ` Pali Rohár
  2024-10-28 11:03 ` [PATCH 4/5] cifs: Fix parsing reparse point with native symlink in SMB1 non-UNICODE session Pali Rohár
  2024-10-28 11:03 ` [PATCH 5/5] cifs: Remove unicode parameter from parse_reparse_point() function Pali Rohár
  4 siblings, 0 replies; 8+ messages in thread
From: Pali Rohár @ 2024-10-28 11:03 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

SMB1 Session Setup NTLMSSP Request in non-UNICODE mode is similar to
UNICODE mode, just strings are encoded in ASCII and not in UTF-16.

With this change it is possible to setup SMB1 session with NTLM
authentication in non-UNICODE mode with Windows SMB server.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/sess.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
index 7226f2563b99..29a1756ba0f7 100644
--- a/fs/smb/client/sess.c
+++ b/fs/smb/client/sess.c
@@ -1704,22 +1704,22 @@ _sess_auth_rawntlmssp_assemble_req(struct sess_data *sess_data)
 	pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
 
 	capabilities = cifs_ssetup_hdr(ses, server, pSMB);
-	if ((pSMB->req.hdr.Flags2 & SMBFLG2_UNICODE) == 0) {
-		cifs_dbg(VFS, "NTLMSSP requires Unicode support\n");
-		return -ENOSYS;
-	}
-
 	pSMB->req.hdr.Flags2 |= SMBFLG2_EXT_SEC;
 	capabilities |= CAP_EXTENDED_SECURITY;
 	pSMB->req.Capabilities |= cpu_to_le32(capabilities);
 
 	bcc_ptr = sess_data->iov[2].iov_base;
-	/* unicode strings must be word aligned */
-	if (!IS_ALIGNED(sess_data->iov[0].iov_len + sess_data->iov[1].iov_len, 2)) {
-		*bcc_ptr = 0;
-		bcc_ptr++;
+
+	if (pSMB->req.hdr.Flags2 & SMBFLG2_UNICODE) {
+		/* unicode strings must be word aligned */
+		if (!IS_ALIGNED(sess_data->iov[0].iov_len + sess_data->iov[1].iov_len, 2)) {
+			*bcc_ptr = 0;
+			bcc_ptr++;
+		}
+		unicode_oslm_strings(&bcc_ptr, sess_data->nls_cp);
+	} else {
+		ascii_oslm_strings(&bcc_ptr, sess_data->nls_cp);
 	}
-	unicode_oslm_strings(&bcc_ptr, sess_data->nls_cp);
 
 	sess_data->iov[2].iov_len = (long) bcc_ptr -
 					(long) sess_data->iov[2].iov_base;
-- 
2.20.1


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

* [PATCH 4/5] cifs: Fix parsing reparse point with native symlink in SMB1 non-UNICODE session
  2024-10-28 11:03 [PATCH 0/5] cifs: Fixes for SMB1 non-UNICODE 8-bit mode Pali Rohár
                   ` (2 preceding siblings ...)
  2024-10-28 11:03 ` [PATCH 3/5] cifs: Add support for SMB1 Session Setup NTLMSSP " Pali Rohár
@ 2024-10-28 11:03 ` Pali Rohár
  2024-10-28 11:03 ` [PATCH 5/5] cifs: Remove unicode parameter from parse_reparse_point() function Pali Rohár
  4 siblings, 0 replies; 8+ messages in thread
From: Pali Rohár @ 2024-10-28 11:03 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

SMB1 NT_TRANSACT_IOCTL/FSCTL_GET_REPARSE_POINT even in non-UNICODE mode
returns reparse buffer in UNICODE/UTF-16 format.

This is because FSCTL_GET_REPARSE_POINT is NT-based IOCTL which does not
distinguish between 8-bit non-UNICODE and 16-bit UNICODE modes and its path
buffers are always encoded in UTF-16.

This change fixes reading of native symlinks in SMB1 when UNICODE session
is not active.

Fixes: ed3e0a149b58 ("smb: client: implement ->query_reparse_point() for SMB1")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/smb1ops.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index b0fb4a5c586d..f552951cfc04 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -1000,12 +1000,11 @@ static int cifs_parse_reparse_point(struct cifs_sb_info *cifs_sb,
 {
 	struct reparse_data_buffer *buf;
 	TRANSACT_IOCTL_RSP *io = rsp_iov->iov_base;
-	bool unicode = !!(io->hdr.Flags2 & SMBFLG2_UNICODE);
 	u32 plen = le16_to_cpu(io->ByteCount);
 
 	buf = (struct reparse_data_buffer *)((__u8 *)&io->hdr.Protocol +
 					     le32_to_cpu(io->DataOffset));
-	return parse_reparse_point(buf, plen, cifs_sb, full_path, unicode, data);
+	return parse_reparse_point(buf, plen, cifs_sb, full_path, true, data);
 }
 
 static bool
-- 
2.20.1


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

* [PATCH 5/5] cifs: Remove unicode parameter from parse_reparse_point() function
  2024-10-28 11:03 [PATCH 0/5] cifs: Fixes for SMB1 non-UNICODE 8-bit mode Pali Rohár
                   ` (3 preceding siblings ...)
  2024-10-28 11:03 ` [PATCH 4/5] cifs: Fix parsing reparse point with native symlink in SMB1 non-UNICODE session Pali Rohár
@ 2024-10-28 11:03 ` Pali Rohár
  4 siblings, 0 replies; 8+ messages in thread
From: Pali Rohár @ 2024-10-28 11:03 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

This parameter is always true, so remove it and also remove dead code which
is never called (for all false code paths).

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifsproto.h |  2 +-
 fs/smb/client/reparse.c   | 25 +++++++++++--------------
 fs/smb/client/smb1ops.c   |  2 +-
 fs/smb/client/smb2file.c  |  1 -
 fs/smb/client/smb2proto.h |  2 +-
 5 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index a7906c193647..31445d40f179 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -667,7 +667,7 @@ char *extract_sharename(const char *unc);
 int parse_reparse_point(struct reparse_data_buffer *buf,
 			u32 plen, struct cifs_sb_info *cifs_sb,
 			const char *full_path,
-			bool unicode, struct cifs_open_info_data *data);
+			struct cifs_open_info_data *data);
 int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 			 struct dentry *dentry, struct cifs_tcon *tcon,
 			 const char *full_path, umode_t mode, dev_t dev,
diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index 3de91331c96c..50720142813e 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -775,7 +775,7 @@ static int parse_reparse_posix(struct reparse_posix_data *buf,
 }
 
 int smb2_parse_native_symlink(char **target, const char *buf, unsigned int len,
-			      bool unicode, bool relative,
+			      bool relative,
 			      const char *full_path,
 			      struct cifs_sb_info *cifs_sb)
 {
@@ -789,26 +789,24 @@ int smb2_parse_native_symlink(char **target, const char *buf, unsigned int len,
 	int rc;
 	int i;
 
-	/* Check that length it valid for unicode/non-unicode mode */
-	if (!len || (unicode && (len % 2))) {
+	/* Check that length it valid */
+	if (!len || (len % 2)) {
 		cifs_dbg(VFS, "srv returned malformed symlink buffer\n");
 		rc = -EIO;
 		goto out;
 	}
 
 	/*
-	 * Check that buffer does not contain UTF-16 null codepoint in unicode
-	 * mode or null byte in non-unicode mode because Linux cannot process
-	 * symlink with null byte.
+	 * Check that buffer does not contain UTF-16 null codepoint
+	 * because Linux cannot process symlink with null byte.
 	 */
-	if ((unicode && UniStrnlen((wchar_t *)buf, len/2) != len/2) ||
-	    (!unicode && strnlen(buf, len) != len)) {
+	if (UniStrnlen((wchar_t *)buf, len/2) != len/2) {
 		cifs_dbg(VFS, "srv returned null byte in native symlink target location\n");
 		rc = -EIO;
 		goto out;
 	}
 
-	smb_target = cifs_strndup_from_utf16(buf, len, unicode, cifs_sb->local_nls);
+	smb_target = cifs_strndup_from_utf16(buf, len, true, cifs_sb->local_nls);
 	if (!smb_target) {
 		rc = -ENOMEM;
 		goto out;
@@ -985,7 +983,7 @@ int smb2_parse_native_symlink(char **target, const char *buf, unsigned int len,
 }
 
 static int parse_reparse_native_symlink(struct reparse_symlink_data_buffer *sym,
-				 u32 plen, bool unicode,
+				 u32 plen,
 				 struct cifs_sb_info *cifs_sb,
 				 const char *full_path,
 				 struct cifs_open_info_data *data)
@@ -1005,7 +1003,6 @@ static int parse_reparse_native_symlink(struct reparse_symlink_data_buffer *sym,
 	return smb2_parse_native_symlink(&data->symlink_target,
 					 sym->PathBuffer + offs,
 					 len,
-					 unicode,
 					 le32_to_cpu(sym->Flags) & SYMLINK_FLAG_RELATIVE,
 					 full_path,
 					 cifs_sb);
@@ -1060,7 +1057,7 @@ static int parse_reparse_wsl_symlink(struct reparse_wsl_symlink_data_buffer *buf
 int parse_reparse_point(struct reparse_data_buffer *buf,
 			u32 plen, struct cifs_sb_info *cifs_sb,
 			const char *full_path,
-			bool unicode, struct cifs_open_info_data *data)
+			struct cifs_open_info_data *data)
 {
 	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
 
@@ -1074,7 +1071,7 @@ int parse_reparse_point(struct reparse_data_buffer *buf,
 	case IO_REPARSE_TAG_SYMLINK:
 		return parse_reparse_native_symlink(
 			(struct reparse_symlink_data_buffer *)buf,
-			plen, unicode, cifs_sb, full_path, data);
+			plen, cifs_sb, full_path, data);
 	case IO_REPARSE_TAG_LX_SYMLINK:
 		return parse_reparse_wsl_symlink(
 			(struct reparse_wsl_symlink_data_buffer *)buf,
@@ -1108,7 +1105,7 @@ int smb2_parse_reparse_point(struct cifs_sb_info *cifs_sb,
 
 	buf = (struct reparse_data_buffer *)((u8 *)io +
 					     le32_to_cpu(io->OutputOffset));
-	return parse_reparse_point(buf, plen, cifs_sb, full_path, true, data);
+	return parse_reparse_point(buf, plen, cifs_sb, full_path, data);
 }
 
 static void wsl_to_fattr(struct cifs_open_info_data *data,
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index f552951cfc04..3667fb94cbf5 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -1004,7 +1004,7 @@ static int cifs_parse_reparse_point(struct cifs_sb_info *cifs_sb,
 
 	buf = (struct reparse_data_buffer *)((__u8 *)&io->hdr.Protocol +
 					     le32_to_cpu(io->DataOffset));
-	return parse_reparse_point(buf, plen, cifs_sb, full_path, true, data);
+	return parse_reparse_point(buf, plen, cifs_sb, full_path, data);
 }
 
 static bool
diff --git a/fs/smb/client/smb2file.c b/fs/smb/client/smb2file.c
index 91b8d00fb88e..b0037058e8d9 100644
--- a/fs/smb/client/smb2file.c
+++ b/fs/smb/client/smb2file.c
@@ -132,7 +132,6 @@ int smb2_parse_symlink_response(struct cifs_sb_info *cifs_sb, const struct kvec
 	return smb2_parse_native_symlink(path,
 					 (char *)sym->PathBuffer + sub_offs,
 					 sub_len,
-					 true,
 					 le32_to_cpu(sym->Flags) & SYMLINK_FLAG_RELATIVE,
 					 full_path,
 					 cifs_sb);
diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h
index 5390d5a61039..c62493c0afc7 100644
--- a/fs/smb/client/smb2proto.h
+++ b/fs/smb/client/smb2proto.h
@@ -115,7 +115,7 @@ extern int smb3_query_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
 			  unsigned int *pbytes_read);
 int smb2_fix_symlink_target_type(char **target, bool directory, struct cifs_sb_info *cifs_sb);
 int smb2_parse_native_symlink(char **target, const char *buf, unsigned int len,
-			      bool unicode, bool relative,
+			      bool relative,
 			      const char *full_path,
 			      struct cifs_sb_info *cifs_sb);
 int smb2_parse_symlink_response(struct cifs_sb_info *cifs_sb,
-- 
2.20.1


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

* Re: [PATCH 1/5] cifs: Add new mount option -o nounicode to disable SMB1 UNICODE mode
  2024-10-28 11:03 ` [PATCH 1/5] cifs: Add new mount option -o nounicode to disable SMB1 UNICODE mode Pali Rohár
@ 2024-11-15 13:26   ` Björn JACKE
  2024-11-15 13:33     ` Pali Rohár
  0 siblings, 1 reply; 8+ messages in thread
From: Björn JACKE @ 2024-11-15 13:26 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, linux-cifs,
	linux-kernel

you can  simply use iocharset=iso8859-1 mount option if you want to get legacy
charsets on the client side to work. I don't think that it's a good idea to
mess with the ancient non-unicode flag of SMB.

Björn

On 2024-10-28 at 12:03 +0100 Pali Rohár sent off:
> SMB1 protocol supports non-UNICODE (8-bit OEM character set) and
> UNICODE (UTF-16) modes.
> 
> Linux SMB1 client implements both of them but currently does not allow to
> choose non-UNICODE mode when SMB1 server announce UNICODE mode support.
> 
> This change adds a new mount option -o nounicode to disable UNICODE mode
> and force usage of non-UNICODE (8-bit OEM character set) mode.
> 
> This allows to test non-UNICODE implementation of Linux SMB1 client against
> any SMB1 server, including modern and recent Windows SMB1 server.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  fs/smb/client/cifsfs.c     |  4 ++++
>  fs/smb/client/cifsglob.h   |  2 ++
>  fs/smb/client/cifssmb.c    |  5 ++++-
>  fs/smb/client/connect.c    | 32 +++++++++++++++++++++++++++++---
>  fs/smb/client/fs_context.c | 11 +++++++++++
>  fs/smb/client/fs_context.h |  2 ++
>  fs/smb/client/sess.c       |  1 +
>  fs/smb/client/smb1ops.c    |  1 +
>  8 files changed, 54 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
> index 1decf91d3f61..447ed7831f2c 100644
> --- a/fs/smb/client/cifsfs.c
> +++ b/fs/smb/client/cifsfs.c
> @@ -611,6 +611,10 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
>  					   cifs_sb->ctx->dir_mode);
>  	if (cifs_sb->ctx->iocharset)
>  		seq_printf(s, ",iocharset=%s", cifs_sb->ctx->iocharset);
> +	if (tcon->ses->unicode == 0)
> +		seq_puts(s, ",nounicode");
> +	else if (tcon->ses->unicode == 1)
> +		seq_puts(s, ",unicode");
>  	if (tcon->seal)
>  		seq_puts(s, ",seal");
>  	else if (tcon->ses->server->ignore_signature)
> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> index dcee43889358..2f77b558abe8 100644
> --- a/fs/smb/client/cifsglob.h
> +++ b/fs/smb/client/cifsglob.h
> @@ -651,6 +651,7 @@ struct smb_version_values {
>  	unsigned int	cap_unix;
>  	unsigned int	cap_nt_find;
>  	unsigned int	cap_large_files;
> +	unsigned int	cap_unicode;
>  	__u16		signing_enabled;
>  	__u16		signing_required;
>  	size_t		create_lease_size;
> @@ -1124,6 +1125,7 @@ struct cifs_ses {
>  	bool sign;		/* is signing required? */
>  	bool domainAuto:1;
>  	bool expired_pwd;  /* track if access denied or expired pwd so can know if need to update */
> +	int unicode;
>  	unsigned int flags;
>  	__u16 session_flags;
>  	__u8 smb3signingkey[SMB3_SIGN_KEY_SIZE];
> diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
> index c6f15dbe860a..6218b59b9da7 100644
> --- a/fs/smb/client/cifssmb.c
> +++ b/fs/smb/client/cifssmb.c
> @@ -423,7 +423,10 @@ CIFSSMBNegotiate(const unsigned int xid,
>  		return rc;
>  
>  	pSMB->hdr.Mid = get_next_mid(server);
> -	pSMB->hdr.Flags2 |= (SMBFLG2_UNICODE | SMBFLG2_ERR_STATUS);
> +	pSMB->hdr.Flags2 |= SMBFLG2_ERR_STATUS;
> +
> +	if (ses->unicode != 0)
> +		pSMB->hdr.Flags2 |= SMBFLG2_UNICODE;
>  
>  	if (should_set_ext_sec_flag(ses->sectype)) {
>  		cifs_dbg(FYI, "Requesting extended security\n");
> diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> index 612816ec71f5..3d9d736b6e58 100644
> --- a/fs/smb/client/connect.c
> +++ b/fs/smb/client/connect.c
> @@ -2338,6 +2338,7 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
>  	ses->cred_uid = ctx->cred_uid;
>  	ses->linux_uid = ctx->linux_uid;
>  
> +	ses->unicode = ctx->unicode;
>  	ses->sectype = ctx->sectype;
>  	ses->sign = ctx->sign;
>  	ses->local_nls = load_nls(ctx->local_nls->charset);
> @@ -3928,7 +3929,7 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
>  		   struct TCP_Server_Info *server,
>  		   struct nls_table *nls_info)
>  {
> -	int rc = -ENOSYS;
> +	int rc = 0;
>  	struct TCP_Server_Info *pserver = SERVER_IS_CHAN(server) ? server->primary_server : server;
>  	struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)&pserver->dstaddr;
>  	struct sockaddr_in *addr = (struct sockaddr_in *)&pserver->dstaddr;
> @@ -3980,6 +3981,26 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
>  		if (!linuxExtEnabled)
>  			ses->capabilities &= (~server->vals->cap_unix);
>  
> +		/*
> +		 * Check if the server supports specified encoding mode.
> +		 * Zero value in vals->cap_unicode indidcates that chosen
> +		 * protocol dialect does not support non-UNICODE mode.
> +		 */
> +		if (ses->unicode == 1 && server->vals->cap_unicode != 0 &&
> +		    !(server->capabilities & server->vals->cap_unicode)) {
> +			cifs_dbg(VFS, "Server does not support mounting in UNICODE mode\n");
> +			rc = -EOPNOTSUPP;
> +		} else if (ses->unicode == 0 && server->vals->cap_unicode == 0) {
> +			cifs_dbg(VFS, "Server does not support mounting in non-UNICODE mode\n");
> +			rc = -EOPNOTSUPP;
> +		} else if (ses->unicode == 0) {
> +			/*
> +			 * When UNICODE mode was explicitly disabled then
> +			 * do not announce client UNICODE capability.
> +			 */
> +			ses->capabilities &= (~server->vals->cap_unicode);
> +		}
> +
>  		if (ses->auth_key.response) {
>  			cifs_dbg(FYI, "Free previous auth_key.response = %p\n",
>  				 ses->auth_key.response);
> @@ -3992,8 +4013,12 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
>  	cifs_dbg(FYI, "Security Mode: 0x%x Capabilities: 0x%x TimeAdjust: %d\n",
>  		 server->sec_mode, server->capabilities, server->timeAdj);
>  
> -	if (server->ops->sess_setup)
> -		rc = server->ops->sess_setup(xid, ses, server, nls_info);
> +	if (!rc) {
> +		if (server->ops->sess_setup)
> +			rc = server->ops->sess_setup(xid, ses, server, nls_info);
> +		else
> +			rc = -ENOSYS;
> +	}
>  
>  	if (rc) {
>  		cifs_server_dbg(VFS, "Send error in SessSetup = %d\n", rc);
> @@ -4063,6 +4088,7 @@ cifs_construct_tcon(struct cifs_sb_info *cifs_sb, kuid_t fsuid)
>  	ctx->seal = master_tcon->seal;
>  	ctx->witness = master_tcon->use_witness;
>  	ctx->dfs_root_ses = master_tcon->ses->dfs_root_ses;
> +	ctx->unicode = master_tcon->ses->unicode;
>  
>  	rc = cifs_set_vol_auth(ctx, master_tcon->ses);
>  	if (rc) {
> diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
> index 7dd46cfc9fb3..d970b66e529c 100644
> --- a/fs/smb/client/fs_context.c
> +++ b/fs/smb/client/fs_context.c
> @@ -127,6 +127,7 @@ const struct fs_parameter_spec smb3_fs_parameters[] = {
>  	fsparam_flag("rootfs", Opt_rootfs),
>  	fsparam_flag("compress", Opt_compress),
>  	fsparam_flag("witness", Opt_witness),
> +	fsparam_flag_no("unicode", Opt_unicode),
>  
>  	/* Mount options which take uid or gid */
>  	fsparam_uid("backupuid", Opt_backupuid),
> @@ -930,6 +931,10 @@ static int smb3_verify_reconfigure_ctx(struct fs_context *fc,
>  		cifs_errorf(fc, "can not change iocharset during remount\n");
>  		return -EINVAL;
>  	}
> +	if (new_ctx->unicode != old_ctx->unicode) {
> +		cifs_errorf(fc, "can not change unicode during remount\n");
> +		return -EINVAL;
> +	}
>  
>  	return 0;
>  }
> @@ -1520,6 +1525,10 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
>  		ctx->witness = true;
>  		pr_warn_once("Witness protocol support is experimental\n");
>  		break;
> +	case Opt_unicode:
> +		ctx->unicode = !result.negated;
> +		cifs_dbg(FYI, "unicode set to %d\n", ctx->unicode);
> +		break;
>  	case Opt_rootfs:
>  #ifndef CONFIG_CIFS_ROOT
>  		cifs_dbg(VFS, "rootfs support requires CONFIG_CIFS_ROOT config option\n");
> @@ -1816,6 +1825,8 @@ int smb3_init_fs_context(struct fs_context *fc)
>  	ctx->symlink_type = CIFS_SYMLINK_TYPE_DEFAULT;
>  	ctx->nonativesocket = 0;
>  
> +	ctx->unicode = -1; /* autodetect, but prefer UNICODE mode */
> +
>  /*
>   *	short int override_uid = -1;
>   *	short int override_gid = -1;
> diff --git a/fs/smb/client/fs_context.h b/fs/smb/client/fs_context.h
> index 18d39d457145..1514e05e6629 100644
> --- a/fs/smb/client/fs_context.h
> +++ b/fs/smb/client/fs_context.h
> @@ -127,6 +127,7 @@ enum cifs_param {
>  	Opt_multichannel,
>  	Opt_compress,
>  	Opt_witness,
> +	Opt_unicode,
>  
>  	/* Mount options which take numeric value */
>  	Opt_backupuid,
> @@ -296,6 +297,7 @@ struct smb3_fs_context {
>  	bool compress; /* enable SMB2 messages (READ/WRITE) de/compression */
>  	bool rootfs:1; /* if it's a SMB root file system */
>  	bool witness:1; /* use witness protocol */
> +	int unicode;
>  	char *leaf_fullpath;
>  	struct cifs_ses *dfs_root_ses;
>  	bool dfs_automount:1; /* set for dfs automount only */
> diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
> index c88e9657f47a..6a92db287843 100644
> --- a/fs/smb/client/sess.c
> +++ b/fs/smb/client/sess.c
> @@ -529,6 +529,7 @@ cifs_ses_add_channel(struct cifs_ses *ses,
>  	ctx->password = ses->password;
>  	ctx->sectype = ses->sectype;
>  	ctx->sign = ses->sign;
> +	ctx->unicode = ses->unicode;
>  
>  	/* UNC and paths */
>  	/* XXX: Use ses->server->hostname? */
> diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
> index abca214f923c..b0fb4a5c586d 100644
> --- a/fs/smb/client/smb1ops.c
> +++ b/fs/smb/client/smb1ops.c
> @@ -1189,6 +1189,7 @@ struct smb_version_values smb1_values = {
>  	.cap_unix = CAP_UNIX,
>  	.cap_nt_find = CAP_NT_SMBS | CAP_NT_FIND,
>  	.cap_large_files = CAP_LARGE_FILES,
> +	.cap_unicode = CAP_UNICODE,
>  	.signing_enabled = SECMODE_SIGN_ENABLED,
>  	.signing_required = SECMODE_SIGN_REQUIRED,
>  };
> -- 
> 2.20.1
> 
> 

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

* Re: [PATCH 1/5] cifs: Add new mount option -o nounicode to disable SMB1 UNICODE mode
  2024-11-15 13:26   ` Björn JACKE
@ 2024-11-15 13:33     ` Pali Rohár
  0 siblings, 0 replies; 8+ messages in thread
From: Pali Rohár @ 2024-11-15 13:33 UTC (permalink / raw)
  To: Björn JACKE
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, linux-cifs,
	linux-kernel

Hello Björn, I know about iocharset=iso8859-1 mount option, but change
here is something different. As I wrote in the commit message it allows
to easily test that "non-unicode flag of SMB" code path implementation
in cifs.ko that works correctly. For example, thanks to this change I
was able to find few bugs (including parsing of symlinks) in that code
path and fixes for them are in this patch series.

On Friday 15 November 2024 14:26:19 Björn JACKE wrote:
> you can  simply use iocharset=iso8859-1 mount option if you want to get legacy
> charsets on the client side to work. I don't think that it's a good idea to
> mess with the ancient non-unicode flag of SMB.
> 
> Björn
> 
> On 2024-10-28 at 12:03 +0100 Pali Rohár sent off:
> > SMB1 protocol supports non-UNICODE (8-bit OEM character set) and
> > UNICODE (UTF-16) modes.
> > 
> > Linux SMB1 client implements both of them but currently does not allow to
> > choose non-UNICODE mode when SMB1 server announce UNICODE mode support.
> > 
> > This change adds a new mount option -o nounicode to disable UNICODE mode
> > and force usage of non-UNICODE (8-bit OEM character set) mode.
> > 
> > This allows to test non-UNICODE implementation of Linux SMB1 client against
> > any SMB1 server, including modern and recent Windows SMB1 server.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  fs/smb/client/cifsfs.c     |  4 ++++
> >  fs/smb/client/cifsglob.h   |  2 ++
> >  fs/smb/client/cifssmb.c    |  5 ++++-
> >  fs/smb/client/connect.c    | 32 +++++++++++++++++++++++++++++---
> >  fs/smb/client/fs_context.c | 11 +++++++++++
> >  fs/smb/client/fs_context.h |  2 ++
> >  fs/smb/client/sess.c       |  1 +
> >  fs/smb/client/smb1ops.c    |  1 +
> >  8 files changed, 54 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
> > index 1decf91d3f61..447ed7831f2c 100644
> > --- a/fs/smb/client/cifsfs.c
> > +++ b/fs/smb/client/cifsfs.c
> > @@ -611,6 +611,10 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
> >  					   cifs_sb->ctx->dir_mode);
> >  	if (cifs_sb->ctx->iocharset)
> >  		seq_printf(s, ",iocharset=%s", cifs_sb->ctx->iocharset);
> > +	if (tcon->ses->unicode == 0)
> > +		seq_puts(s, ",nounicode");
> > +	else if (tcon->ses->unicode == 1)
> > +		seq_puts(s, ",unicode");
> >  	if (tcon->seal)
> >  		seq_puts(s, ",seal");
> >  	else if (tcon->ses->server->ignore_signature)
> > diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> > index dcee43889358..2f77b558abe8 100644
> > --- a/fs/smb/client/cifsglob.h
> > +++ b/fs/smb/client/cifsglob.h
> > @@ -651,6 +651,7 @@ struct smb_version_values {
> >  	unsigned int	cap_unix;
> >  	unsigned int	cap_nt_find;
> >  	unsigned int	cap_large_files;
> > +	unsigned int	cap_unicode;
> >  	__u16		signing_enabled;
> >  	__u16		signing_required;
> >  	size_t		create_lease_size;
> > @@ -1124,6 +1125,7 @@ struct cifs_ses {
> >  	bool sign;		/* is signing required? */
> >  	bool domainAuto:1;
> >  	bool expired_pwd;  /* track if access denied or expired pwd so can know if need to update */
> > +	int unicode;
> >  	unsigned int flags;
> >  	__u16 session_flags;
> >  	__u8 smb3signingkey[SMB3_SIGN_KEY_SIZE];
> > diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
> > index c6f15dbe860a..6218b59b9da7 100644
> > --- a/fs/smb/client/cifssmb.c
> > +++ b/fs/smb/client/cifssmb.c
> > @@ -423,7 +423,10 @@ CIFSSMBNegotiate(const unsigned int xid,
> >  		return rc;
> >  
> >  	pSMB->hdr.Mid = get_next_mid(server);
> > -	pSMB->hdr.Flags2 |= (SMBFLG2_UNICODE | SMBFLG2_ERR_STATUS);
> > +	pSMB->hdr.Flags2 |= SMBFLG2_ERR_STATUS;
> > +
> > +	if (ses->unicode != 0)
> > +		pSMB->hdr.Flags2 |= SMBFLG2_UNICODE;
> >  
> >  	if (should_set_ext_sec_flag(ses->sectype)) {
> >  		cifs_dbg(FYI, "Requesting extended security\n");
> > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> > index 612816ec71f5..3d9d736b6e58 100644
> > --- a/fs/smb/client/connect.c
> > +++ b/fs/smb/client/connect.c
> > @@ -2338,6 +2338,7 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
> >  	ses->cred_uid = ctx->cred_uid;
> >  	ses->linux_uid = ctx->linux_uid;
> >  
> > +	ses->unicode = ctx->unicode;
> >  	ses->sectype = ctx->sectype;
> >  	ses->sign = ctx->sign;
> >  	ses->local_nls = load_nls(ctx->local_nls->charset);
> > @@ -3928,7 +3929,7 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
> >  		   struct TCP_Server_Info *server,
> >  		   struct nls_table *nls_info)
> >  {
> > -	int rc = -ENOSYS;
> > +	int rc = 0;
> >  	struct TCP_Server_Info *pserver = SERVER_IS_CHAN(server) ? server->primary_server : server;
> >  	struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)&pserver->dstaddr;
> >  	struct sockaddr_in *addr = (struct sockaddr_in *)&pserver->dstaddr;
> > @@ -3980,6 +3981,26 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
> >  		if (!linuxExtEnabled)
> >  			ses->capabilities &= (~server->vals->cap_unix);
> >  
> > +		/*
> > +		 * Check if the server supports specified encoding mode.
> > +		 * Zero value in vals->cap_unicode indidcates that chosen
> > +		 * protocol dialect does not support non-UNICODE mode.
> > +		 */
> > +		if (ses->unicode == 1 && server->vals->cap_unicode != 0 &&
> > +		    !(server->capabilities & server->vals->cap_unicode)) {
> > +			cifs_dbg(VFS, "Server does not support mounting in UNICODE mode\n");
> > +			rc = -EOPNOTSUPP;
> > +		} else if (ses->unicode == 0 && server->vals->cap_unicode == 0) {
> > +			cifs_dbg(VFS, "Server does not support mounting in non-UNICODE mode\n");
> > +			rc = -EOPNOTSUPP;
> > +		} else if (ses->unicode == 0) {
> > +			/*
> > +			 * When UNICODE mode was explicitly disabled then
> > +			 * do not announce client UNICODE capability.
> > +			 */
> > +			ses->capabilities &= (~server->vals->cap_unicode);
> > +		}
> > +
> >  		if (ses->auth_key.response) {
> >  			cifs_dbg(FYI, "Free previous auth_key.response = %p\n",
> >  				 ses->auth_key.response);
> > @@ -3992,8 +4013,12 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
> >  	cifs_dbg(FYI, "Security Mode: 0x%x Capabilities: 0x%x TimeAdjust: %d\n",
> >  		 server->sec_mode, server->capabilities, server->timeAdj);
> >  
> > -	if (server->ops->sess_setup)
> > -		rc = server->ops->sess_setup(xid, ses, server, nls_info);
> > +	if (!rc) {
> > +		if (server->ops->sess_setup)
> > +			rc = server->ops->sess_setup(xid, ses, server, nls_info);
> > +		else
> > +			rc = -ENOSYS;
> > +	}
> >  
> >  	if (rc) {
> >  		cifs_server_dbg(VFS, "Send error in SessSetup = %d\n", rc);
> > @@ -4063,6 +4088,7 @@ cifs_construct_tcon(struct cifs_sb_info *cifs_sb, kuid_t fsuid)
> >  	ctx->seal = master_tcon->seal;
> >  	ctx->witness = master_tcon->use_witness;
> >  	ctx->dfs_root_ses = master_tcon->ses->dfs_root_ses;
> > +	ctx->unicode = master_tcon->ses->unicode;
> >  
> >  	rc = cifs_set_vol_auth(ctx, master_tcon->ses);
> >  	if (rc) {
> > diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
> > index 7dd46cfc9fb3..d970b66e529c 100644
> > --- a/fs/smb/client/fs_context.c
> > +++ b/fs/smb/client/fs_context.c
> > @@ -127,6 +127,7 @@ const struct fs_parameter_spec smb3_fs_parameters[] = {
> >  	fsparam_flag("rootfs", Opt_rootfs),
> >  	fsparam_flag("compress", Opt_compress),
> >  	fsparam_flag("witness", Opt_witness),
> > +	fsparam_flag_no("unicode", Opt_unicode),
> >  
> >  	/* Mount options which take uid or gid */
> >  	fsparam_uid("backupuid", Opt_backupuid),
> > @@ -930,6 +931,10 @@ static int smb3_verify_reconfigure_ctx(struct fs_context *fc,
> >  		cifs_errorf(fc, "can not change iocharset during remount\n");
> >  		return -EINVAL;
> >  	}
> > +	if (new_ctx->unicode != old_ctx->unicode) {
> > +		cifs_errorf(fc, "can not change unicode during remount\n");
> > +		return -EINVAL;
> > +	}
> >  
> >  	return 0;
> >  }
> > @@ -1520,6 +1525,10 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
> >  		ctx->witness = true;
> >  		pr_warn_once("Witness protocol support is experimental\n");
> >  		break;
> > +	case Opt_unicode:
> > +		ctx->unicode = !result.negated;
> > +		cifs_dbg(FYI, "unicode set to %d\n", ctx->unicode);
> > +		break;
> >  	case Opt_rootfs:
> >  #ifndef CONFIG_CIFS_ROOT
> >  		cifs_dbg(VFS, "rootfs support requires CONFIG_CIFS_ROOT config option\n");
> > @@ -1816,6 +1825,8 @@ int smb3_init_fs_context(struct fs_context *fc)
> >  	ctx->symlink_type = CIFS_SYMLINK_TYPE_DEFAULT;
> >  	ctx->nonativesocket = 0;
> >  
> > +	ctx->unicode = -1; /* autodetect, but prefer UNICODE mode */
> > +
> >  /*
> >   *	short int override_uid = -1;
> >   *	short int override_gid = -1;
> > diff --git a/fs/smb/client/fs_context.h b/fs/smb/client/fs_context.h
> > index 18d39d457145..1514e05e6629 100644
> > --- a/fs/smb/client/fs_context.h
> > +++ b/fs/smb/client/fs_context.h
> > @@ -127,6 +127,7 @@ enum cifs_param {
> >  	Opt_multichannel,
> >  	Opt_compress,
> >  	Opt_witness,
> > +	Opt_unicode,
> >  
> >  	/* Mount options which take numeric value */
> >  	Opt_backupuid,
> > @@ -296,6 +297,7 @@ struct smb3_fs_context {
> >  	bool compress; /* enable SMB2 messages (READ/WRITE) de/compression */
> >  	bool rootfs:1; /* if it's a SMB root file system */
> >  	bool witness:1; /* use witness protocol */
> > +	int unicode;
> >  	char *leaf_fullpath;
> >  	struct cifs_ses *dfs_root_ses;
> >  	bool dfs_automount:1; /* set for dfs automount only */
> > diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
> > index c88e9657f47a..6a92db287843 100644
> > --- a/fs/smb/client/sess.c
> > +++ b/fs/smb/client/sess.c
> > @@ -529,6 +529,7 @@ cifs_ses_add_channel(struct cifs_ses *ses,
> >  	ctx->password = ses->password;
> >  	ctx->sectype = ses->sectype;
> >  	ctx->sign = ses->sign;
> > +	ctx->unicode = ses->unicode;
> >  
> >  	/* UNC and paths */
> >  	/* XXX: Use ses->server->hostname? */
> > diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
> > index abca214f923c..b0fb4a5c586d 100644
> > --- a/fs/smb/client/smb1ops.c
> > +++ b/fs/smb/client/smb1ops.c
> > @@ -1189,6 +1189,7 @@ struct smb_version_values smb1_values = {
> >  	.cap_unix = CAP_UNIX,
> >  	.cap_nt_find = CAP_NT_SMBS | CAP_NT_FIND,
> >  	.cap_large_files = CAP_LARGE_FILES,
> > +	.cap_unicode = CAP_UNICODE,
> >  	.signing_enabled = SECMODE_SIGN_ENABLED,
> >  	.signing_required = SECMODE_SIGN_REQUIRED,
> >  };
> > -- 
> > 2.20.1
> > 
> > 

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

end of thread, other threads:[~2024-11-15 13:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-28 11:03 [PATCH 0/5] cifs: Fixes for SMB1 non-UNICODE 8-bit mode Pali Rohár
2024-10-28 11:03 ` [PATCH 1/5] cifs: Add new mount option -o nounicode to disable SMB1 UNICODE mode Pali Rohár
2024-11-15 13:26   ` Björn JACKE
2024-11-15 13:33     ` Pali Rohár
2024-10-28 11:03 ` [PATCH 2/5] cifs: Fix encoding of SMB1 Session Setup Kerberos Request in non-UNICODE mode Pali Rohár
2024-10-28 11:03 ` [PATCH 3/5] cifs: Add support for SMB1 Session Setup NTLMSSP " Pali Rohár
2024-10-28 11:03 ` [PATCH 4/5] cifs: Fix parsing reparse point with native symlink in SMB1 non-UNICODE session Pali Rohár
2024-10-28 11:03 ` [PATCH 5/5] cifs: Remove unicode parameter from parse_reparse_point() function Pali Rohár

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox