Linux CIFS filesystem development
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Steve French <sfrench@samba.org>
Cc: David Howells <dhowells@redhat.com>,
	Paulo Alcantara <pc@manguebit.org>,
	Shyam Prasad N <sprasad@microsoft.com>,
	Tom Talpey <tom@talpey.com>, Stefan Metzmacher <metze@samba.org>,
	Mina Almasry <almasrymina@google.com>,
	linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: [RFC PATCH 19/36] cifs: Try to better handle the "Dynamic" flag in StructureSize2 in SMB2/3
Date: Tue, 19 May 2026 11:21:37 +0100	[thread overview]
Message-ID: <20260519102158.592165-20-dhowells@redhat.com> (raw)
In-Reply-To: <20260519102158.592165-1-dhowells@redhat.com>

The bottom bit of the size field of the op-specific SMB2/3 struct that
immediately follows the smb[23]_hdr struct is a bit weird.  Wireshark
considers it a "dynamic flag" and it would seem that it needs to be masked
off to get the actualy size of the struct - and if there is data following
the struct, it is kind of meaningless.

The adjusted value, however, must still be set that way, and if there is
nothing following the struct, it seems that the message must be padded out
with an extra zero byte so that the message size is no less than the header
including that flag in the StructureSize.

Possibly this is a remnant of someone putting an empty data field at the
end of the struct as "unsigned char Buffer[1];".  But whatever the origin,
there are a bunch of places in which the extra 1 needs to be subtracted
off.

Since subtracting 1 is the rule rather than the exception, return the size
with the bit masked off and add the pad byte if we don't have anything else
to add (Read being the main example of this).

This can then be handled more transparently in future.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Steve French <sfrench@samba.org>
cc: Paulo Alcantara <pc@manguebit.org>
cc: Shyam Prasad N <sprasad@microsoft.com>
cc: Tom Talpey <tom@talpey.com>
cc: linux-cifs@vger.kernel.org
cc: linux-fsdevel@vger.kernel.org
---
 fs/smb/client/smb2pdu.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index ddf127dc27bb..48f56ce73cd3 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -543,7 +543,11 @@ fill_small_buf(__le16 smb2_command, struct cifs_tcon *tcon,
 	smb2_hdr_assemble(&spdu->hdr, smb2_command, tcon, server);
 	spdu->StructureSize2 = cpu_to_le16(parmsize);
 
-	*total_len = parmsize + sizeof(struct smb2_hdr);
+	if (smb2_command == SMB2_READ)
+		*total_len = sizeof(struct smb2_hdr) + parmsize;
+	else
+		*total_len = sizeof(struct smb2_hdr) +
+			(parmsize & ~SMB2_STRUCT_HAS_DYNAMIC_PART);
 }
 
 /*
@@ -1589,7 +1593,7 @@ SMB2_sess_alloc_buffer(struct SMB2_sess_data *sess_data)
 
 	sess_data->iov[0].iov_base = (char *)req;
 	/* 1 for pad */
-	sess_data->iov[0].iov_len = total_len - 1;
+	sess_data->iov[0].iov_len = total_len;
 	/*
 	 * This variable will be used to clear the buffer
 	 * allocated above in case of any error in the calling function.
@@ -2154,7 +2158,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 
 	iov[0].iov_base = (char *)req;
 	/* 1 for pad */
-	iov[0].iov_len = total_len - 1;
+	iov[0].iov_len = total_len;
 
 	/* Testing shows that buffer offset must be at location of Buffer[0] */
 	req->PathOffset = cpu_to_le16(sizeof(struct smb2_tree_connect_req));
@@ -2958,7 +2962,7 @@ int smb311_posix_mkdir(const unsigned int xid, struct inode *inode,
 
 	iov[0].iov_base = (char *)req;
 	/* -1 since last byte is buf[0] which is sent below (path) */
-	iov[0].iov_len = total_len - 1;
+	iov[0].iov_len = total_len;
 
 	req->NameOffset = cpu_to_le16(sizeof(struct smb2_create_req));
 
@@ -3105,7 +3109,7 @@ SMB2_open_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
 
 	iov[0].iov_base = (char *)req;
 	/* -1 since last byte is buf[0] which is sent below (path) */
-	iov[0].iov_len = total_len - 1;
+	iov[0].iov_len = total_len;
 
 	if (oparms->create_options & CREATE_OPTION_READONLY)
 		file_attributes |= ATTR_READONLY;
@@ -3427,12 +3431,12 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
 		req->InputOffset =
 		       cpu_to_le32(offsetof(struct smb2_ioctl_req, Buffer));
 		rqst->rq_nvec = 2;
-		iov[0].iov_len = total_len - 1;
+		iov[0].iov_len = total_len;
 		iov[1].iov_base = in_data_buf;
 		iov[1].iov_len = indatalen;
 	} else {
 		rqst->rq_nvec = 1;
-		iov[0].iov_len = total_len;
+		iov[0].iov_len = total_len + 1;
 	}
 
 	req->OutputOffset = 0;
@@ -3874,7 +3878,7 @@ SMB2_query_info_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
 	if (input_len) {
 		req->InputBufferLength = cpu_to_le32(input_len);
 		/* total_len for smb query request never close to le16 max */
-		req->InputBufferOffset = cpu_to_le16(total_len - 1);
+		req->InputBufferOffset = cpu_to_le16(total_len);
 		memcpy(req->Buffer, input, input_len);
 	}
 
@@ -4579,7 +4583,7 @@ smb2_new_read_req(void **buf, unsigned int *total_len,
 		v1 = (struct smbdirect_buffer_descriptor_v1 *) &req->Buffer[0];
 		smbd_mr_fill_buffer_descriptor(rdata->mr, v1);
 
-		*total_len += sizeof(*v1) - 1;
+		*total_len += sizeof(*v1);
 	}
 #endif
 	if (request_type & CHAINED_REQUEST) {
@@ -5339,7 +5343,7 @@ SMB2_write(const unsigned int xid, struct cifs_io_parms *io_parms,
 
 	iov[0].iov_base = (char *)req;
 	/* 1 for Buffer */
-	iov[0].iov_len = total_len - 1;
+	iov[0].iov_len = total_len;
 
 	memset(&rqst, 0, sizeof(struct smb_rqst));
 	rqst.rq_iov = iov;
@@ -5597,7 +5601,7 @@ int SMB2_query_directory_init(const unsigned int xid,
 
 	iov[0].iov_base = (char *)req;
 	/* 1 for Buffer */
-	iov[0].iov_len = total_len - 1;
+	iov[0].iov_len = total_len;
 
 	iov[1].iov_base = (char *)(req->Buffer);
 	iov[1].iov_len = len;
@@ -5810,7 +5814,7 @@ SMB2_set_info_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
 
 	iov[0].iov_base = (char *)req;
 	/* 1 for Buffer */
-	iov[0].iov_len = total_len - 1;
+	iov[0].iov_len = total_len;
 
 	for (i = 1; i < rqst->rq_nvec; i++) {
 		le32_add_cpu(&req->BufferLength, size[i]);


  parent reply	other threads:[~2026-05-19 10:23 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-19 10:21 [RFC PATCH 00/36] cifs: [WIP] Overhaul message handling and improve nework transport David Howells
2026-05-19 10:21 ` [RFC PATCH 01/36] net: Perform special handling for a splice from a bvecq David Howells
2026-05-19 10:21 ` [RFC PATCH 02/36] netfs: Add a facility to splice TCP receive buffers into " David Howells
2026-05-19 10:21 ` [RFC PATCH 03/36] netfs: Add some TCP receive queue helpers David Howells
2026-05-19 10:21 ` [RFC PATCH 04/36] cifs, nls: Provide unicode size determination func David Howells
2026-05-19 10:21 ` [RFC PATCH 05/36] cifs: Introduce an ALIGN8() macro David Howells
2026-05-19 10:21 ` [RFC PATCH 06/36] cifs: Rename mid_q_entry to smb_message David Howells
2026-05-19 10:21 ` [RFC PATCH 07/36] cifs: Add "Has dynamic part" flag form SMB2/3 StructureSize LSB David Howells
2026-05-19 10:21 ` [RFC PATCH 08/36] cifs: Add an enum to hold a trace value for the command/subcommand David Howells
2026-05-19 10:21 ` [RFC PATCH 09/36] cifs: Institute message managing struct David Howells
2026-05-19 10:21 ` [RFC PATCH 10/36] cifs: Split crypt_message() into encrypt and decrypt variants David Howells
2026-05-19 10:21 ` [RFC PATCH 11/36] cifs: Add new AEAD alloc and setup routines that draw from an iterator David Howells
2026-05-19 10:21 ` [RFC PATCH 12/36] cifs: [WIP] Rewrite base Rx to put data off the socket into a bvecq David Howells
2026-05-19 10:21 ` [RFC PATCH 13/36] cifs: Remove validate_t2() David Howells
2026-05-19 10:21 ` [RFC PATCH 14/36] cifs: Remove cifs_io_subrequest::got_bytes David Howells
2026-05-19 10:21 ` [RFC PATCH 15/36] cifs: Pass smb_message to cifs_verify_signature() David Howells
2026-05-19 10:21 ` [RFC PATCH 16/36] cifs: Rewrite base TCP transmission David Howells
2026-05-19 10:36   ` Stefan Metzmacher
2026-05-19 10:21 ` [RFC PATCH 17/36] cifs: Don't use corking David Howells
2026-05-19 10:21 ` [RFC PATCH 18/36] cifs: Use page frag allocator for Tx buffers David Howells
2026-05-19 10:21 ` David Howells [this message]
2026-05-19 10:21 ` [RFC PATCH 20/36] cifs: Pass smb_message structs down into the transport layer David Howells
2026-05-19 10:21 ` [RFC PATCH 21/36] cifs: Add a tracepoint to trace the smb_message refcount David Howells
2026-05-19 10:21 ` [RFC PATCH 22/36] cifs: Trace smb1/2_copy_to_prepped_buffers() David Howells
2026-05-19 10:21 ` [RFC PATCH 23/36] cifs: Clean up mid->callback_data and kill off mid->creator David Howells
2026-05-19 10:21 ` [RFC PATCH 24/36] cifs: Add netmem allocation functions David Howells
2026-05-19 10:21 ` [RFC PATCH 25/36] cifs: Add more pieces to smb_message David Howells
2026-05-19 10:21 ` [RFC PATCH 26/36] cifs: Convert SMB2 Negotiate Protocol request David Howells
2026-05-19 10:21 ` [RFC PATCH 27/36] cifs: Convert SMB2 Session Setup request David Howells
2026-05-19 10:21 ` [RFC PATCH 28/36] cifs: Convert SMB2 Logoff request David Howells
2026-05-19 10:21 ` [RFC PATCH 29/36] cifs: Convert SMB2 Tree Connect request David Howells
2026-05-19 10:21 ` [RFC PATCH 30/36] cifs: Convert SMB2 Tree Disconnect request David Howells
2026-05-19 10:21 ` [RFC PATCH 31/36] cifs: Convert SMB2 Read request David Howells
2026-05-19 10:21 ` [RFC PATCH 32/36] cifs: Convert SMB2 Write request David Howells
2026-05-19 10:21 ` [RFC PATCH 33/36] cifs: [WIP] Don't copy new-style smb_messages to a set of pages David Howells
2026-05-19 10:21 ` [RFC PATCH 34/36] cifs: [WIP] Rearrange Create request subfuncs David Howells
2026-05-19 10:21 ` [RFC PATCH 35/36] cifs: [WIP] Convert SMB2 Posix Mkdir request David Howells
2026-05-19 10:21 ` [RFC PATCH 36/36] cifs: [WIP] Convert SMB2 Open request David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260519102158.592165-20-dhowells@redhat.com \
    --to=dhowells@redhat.com \
    --cc=almasrymina@google.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=metze@samba.org \
    --cc=pc@manguebit.org \
    --cc=sfrench@samba.org \
    --cc=sprasad@microsoft.com \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox