On 8/30/2022 10:30 AM, huaweicloud wrote:
I think the struct validate_negotiate_info_req is an variable-length array,
just for implement simple, defind as 4 in here.
But that's my point. The code picks "4" arbitrarily, and that
number can change - as you discovered, it already did since
it used to be "3".
smb2pdu.h:
struct validate_negotiate_info_req {
__le32 Capabilities;
__u8 Guid[SMB2_CLIENT_GUID_SIZE];
__le16 SecurityMode;
__le16 DialectCount;
__le16 Dialects[4]; /* BB expand this if autonegotiate > 4 dialects */
} __packed;
I repeat my suggestion.
Alternatively, change the code to make it a variable array and
allocate it properly.
Tom.
According MS-SMB2, there really not define the length of the package, just
define the count of the dialects, but just send the needed data maybe more
appropriate.
Thanks.
在 2022/8/30 22:03, Tom Talpey 写道:
Wouldn't it be safer to just set the size, instead of implicitly
assuming that there are 4 array elements?
inbuflen = sizeof(*pneg_inbuf) - sizeof(pneg_inbuf.Dialects) +
sizeof(pneg_inbuf.Dialects[0]);
Or, because it obviously works to send the extra bytes even
though the DialectCount is just 1, just zero them out?
memset(pneg_inbuf.Dialects, 0, sizeof(pneg_inbuf.Dialects));
pneg_inbuf.Dialects[0] = cpu_to_le16(server->vals->protocol_id);
Tom.
On 8/30/2022 3:06 AM, huaweicloud wrote:
Hi Steve,
Could you help to review this patch.
Thanks,
Zhang Xiaoxu
在 2022/8/24 16:57, Zhang Xiaoxu 写道:
Commit d5c7076b772a ("smb3: add smb3.1.1 to default dialect list")
extend the dialects from 3 to 4, but forget to decrease the extended
length when specific the dialect, then the message length is larger
than expected.
This maybe leak some info through network because not initialize the
message body.
After apply this patch, the VALIDATE_NEGOTIATE_INFO message length is
reduced from 28 bytes to 26 bytes.
Fixes: d5c7076b772a ("smb3: add smb3.1.1 to default dialect list")
Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@xxxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx>
---
fs/cifs/smb2pdu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 1ecc2501c56f..3df7adc01fe5 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1162,9 +1162,9 @@ int smb3_validate_negotiate(const unsigned int
xid, struct cifs_tcon *tcon)
pneg_inbuf->Dialects[0] =
cpu_to_le16(server->vals->protocol_id);
pneg_inbuf->DialectCount = cpu_to_le16(1);
- /* structure is big enough for 3 dialects, sending only 1 */
+ /* structure is big enough for 4 dialects, sending only 1 */
inbuflen = sizeof(*pneg_inbuf) -
- sizeof(pneg_inbuf->Dialects[0]) * 2;
+ sizeof(pneg_inbuf->Dialects[0]) * 3;
}
rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
