Re: [PATCH v6 4/5] cifs: Add neg dialects info to smb version values

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

Steve, your opinion on including this? You may have missed the
question I asked earlier in the thread:


Is this really necessary? It's nice and all, but there aren't
any new SMB2/3 dialects coming down the pike. I'm ambivalent
about changing the code unless there's an actual issue, for
the purpose of maintaining stable, etc. Steve? 

Tom.

On 9/13/2022 7:17 PM, Zhang Xiaoxu wrote:

The dialects information when negotiate with server is
depends on the smb version, add it to the version values
and make code simple.

Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@xxxxxxxxxx>
Acked-by: Tom Talpey <tom@xxxxxxxxxx>
---
  fs/cifs/cifsglob.h |  2 ++
  fs/cifs/smb2ops.c  | 35 ++++++++++++++++++++++++++++
  fs/cifs/smb2pdu.c  | 58 +++++++---------------------------------------
  3 files changed, 46 insertions(+), 49 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index ae7f571a7dba..376421b63738 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -553,6 +553,8 @@ struct smb_version_values {
  	__u16		signing_enabled;
  	__u16		signing_required;
  	size_t		create_lease_size;
+	int		neg_dialect_cnt;
+	__le16		*neg_dialects;
  };
#define HEADER_SIZE(server) (server->vals->header_size) 
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 421be43af425..e1407124c761 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -5664,6 +5664,12 @@ struct smb_version_values smb21_values = {
  	.create_lease_size = sizeof(struct create_lease),
  };
+static __le16 smb3any_neg_dialects[] = { 
+	cpu_to_le16(SMB30_PROT_ID),
+	cpu_to_le16(SMB302_PROT_ID),
+	cpu_to_le16(SMB311_PROT_ID)
+};
+
  struct smb_version_values smb3any_values = {
  	.version_string = SMB3ANY_VERSION_STRING,
  	.protocol_id = SMB302_PROT_ID, /* doesn't matter, send protocol array */
@@ -5683,6 +5689,15 @@ struct smb_version_values smb3any_values = {
  	.signing_enabled = SMB2_NEGOTIATE_SIGNING_ENABLED | SMB2_NEGOTIATE_SIGNING_REQUIRED,
  	.signing_required = SMB2_NEGOTIATE_SIGNING_REQUIRED,
  	.create_lease_size = sizeof(struct create_lease_v2),
+	.neg_dialect_cnt = ARRAY_SIZE(smb3any_neg_dialects),
+	.neg_dialects = smb3any_neg_dialects,
+};
+
+static __le16 smbdefault_neg_dialects[] = {
+	cpu_to_le16(SMB21_PROT_ID),
+	cpu_to_le16(SMB30_PROT_ID),
+	cpu_to_le16(SMB302_PROT_ID),
+	cpu_to_le16(SMB311_PROT_ID)
  };
struct smb_version_values smbdefault_values = { 
@@ -5704,6 +5719,12 @@ struct smb_version_values smbdefault_values = {
  	.signing_enabled = SMB2_NEGOTIATE_SIGNING_ENABLED | SMB2_NEGOTIATE_SIGNING_REQUIRED,
  	.signing_required = SMB2_NEGOTIATE_SIGNING_REQUIRED,
  	.create_lease_size = sizeof(struct create_lease_v2),
+	.neg_dialect_cnt = ARRAY_SIZE(smbdefault_neg_dialects),
+	.neg_dialects = smbdefault_neg_dialects,
+};
+
+static __le16 smb30_neg_dialects[] = {
+	cpu_to_le16(SMB30_PROT_ID),
  };
struct smb_version_values smb30_values = { 
@@ -5725,6 +5746,12 @@ struct smb_version_values smb30_values = {
  	.signing_enabled = SMB2_NEGOTIATE_SIGNING_ENABLED | SMB2_NEGOTIATE_SIGNING_REQUIRED,
  	.signing_required = SMB2_NEGOTIATE_SIGNING_REQUIRED,
  	.create_lease_size = sizeof(struct create_lease_v2),
+	.neg_dialect_cnt = ARRAY_SIZE(smb30_neg_dialects),
+	.neg_dialects = smb30_neg_dialects,
+};
+
+static __le16 smb302_neg_dialects[] = {
+	cpu_to_le16(SMB302_PROT_ID),
  };
struct smb_version_values smb302_values = { 
@@ -5746,6 +5773,12 @@ struct smb_version_values smb302_values = {
  	.signing_enabled = SMB2_NEGOTIATE_SIGNING_ENABLED | SMB2_NEGOTIATE_SIGNING_REQUIRED,
  	.signing_required = SMB2_NEGOTIATE_SIGNING_REQUIRED,
  	.create_lease_size = sizeof(struct create_lease_v2),
+	.neg_dialect_cnt = ARRAY_SIZE(smb302_neg_dialects),
+	.neg_dialects = smb302_neg_dialects,
+};
+
+static __le16 smb311_neg_dialects[] = {
+	cpu_to_le16(SMB311_PROT_ID),
  };
struct smb_version_values smb311_values = { 
@@ -5767,4 +5800,6 @@ struct smb_version_values smb311_values = {
  	.signing_enabled = SMB2_NEGOTIATE_SIGNING_ENABLED | SMB2_NEGOTIATE_SIGNING_REQUIRED,
  	.signing_required = SMB2_NEGOTIATE_SIGNING_REQUIRED,
  	.create_lease_size = sizeof(struct create_lease_v2),
+	.neg_dialect_cnt = ARRAY_SIZE(smb311_neg_dialects),
+	.neg_dialects = smb311_neg_dialects,
  };
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 223056097b54..482ed480fbc6 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -897,27 +897,10 @@ SMB2_negotiate(const unsigned int xid,
  	memset(server->preauth_sha_hash, 0, SMB2_PREAUTH_HASH_SIZE);
  	memset(ses->preauth_sha_hash, 0, SMB2_PREAUTH_HASH_SIZE);
- if (strcmp(server->vals->version_string, 
-		   SMB3ANY_VERSION_STRING) == 0) {
-		req->Dialects[0] = cpu_to_le16(SMB30_PROT_ID);
-		req->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);
-		req->Dialects[2] = cpu_to_le16(SMB311_PROT_ID);
-		req->DialectCount = cpu_to_le16(3);
-		total_len += 6;
-	} else if (strcmp(server->vals->version_string,
-		   SMBDEFAULT_VERSION_STRING) == 0) {
-		req->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
-		req->Dialects[1] = cpu_to_le16(SMB30_PROT_ID);
-		req->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
-		req->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
-		req->DialectCount = cpu_to_le16(4);
-		total_len += 8;
-	} else {
-		/* otherwise send specific dialect */
-		req->Dialects[0] = cpu_to_le16(server->vals->protocol_id);
-		req->DialectCount = cpu_to_le16(1);
-		total_len += 2;
-	}
+	req->DialectCount = cpu_to_le16(server->vals->neg_dialect_cnt);
+	memcpy(req->Dialects, server->vals->neg_dialects,
+		sizeof(__le16) * server->vals->neg_dialect_cnt);
+	total_len += sizeof(__le16) * server->vals->neg_dialect_cnt;
/* only one of SMB2 signing flags may be set in SMB2 request */ 
  	if (ses->sign)
@@ -1145,34 +1128,11 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
  	else
  		pneg_inbuf->SecurityMode = 0;
- 
-	if (strcmp(server->vals->version_string,
-		SMB3ANY_VERSION_STRING) == 0) {
-		pneg_inbuf->Dialects[0] = cpu_to_le16(SMB30_PROT_ID);
-		pneg_inbuf->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);
-		pneg_inbuf->Dialects[2] = cpu_to_le16(SMB311_PROT_ID);
-		pneg_inbuf->DialectCount = cpu_to_le16(3);
-		/* SMB 2.1 not included so subtract one dialect from len */
-		inbuflen = sizeof(*pneg_inbuf) -
-				(sizeof(pneg_inbuf->Dialects[0]));
-	} else if (strcmp(server->vals->version_string,
-		SMBDEFAULT_VERSION_STRING) == 0) {
-		pneg_inbuf->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
-		pneg_inbuf->Dialects[1] = cpu_to_le16(SMB30_PROT_ID);
-		pneg_inbuf->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
-		pneg_inbuf->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
-		pneg_inbuf->DialectCount = cpu_to_le16(4);
-		/* structure is big enough for 4 dialects */
-		inbuflen = sizeof(*pneg_inbuf);
-	} else {
-		/* otherwise specific dialect was requested */
-		pneg_inbuf->Dialects[0] =
-			cpu_to_le16(server->vals->protocol_id);
-		pneg_inbuf->DialectCount = cpu_to_le16(1);
-		/* structure is big enough for 4 dialects, sending only 1 */
-		inbuflen = sizeof(*pneg_inbuf) -
-				sizeof(pneg_inbuf->Dialects[0]) * 3;
-	}
+	pneg_inbuf->DialectCount = cpu_to_le16(server->vals->neg_dialect_cnt);
+	memcpy(pneg_inbuf->Dialects, server->vals->neg_dialects,
+		server->vals->neg_dialect_cnt * sizeof(__le16));
+	inbuflen = offsetof(struct validate_negotiate_info_req, Dialects) +
+		sizeof(pneg_inbuf->Dialects[0]) * server->vals->neg_dialect_cnt;
rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID, 
  		FSCTL_VALIDATE_NEGOTIATE_INFO,
[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux