On 9/1/2022 10:24 AM, Zhang Xiaoxu wrote:
The length of the message FSCTL_VALIDATE_NEGOTIATE_INFO is
depends on the count of the dialects, the dialects count is
depending on the smb version, so the dialects should be
variable array.
Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@xxxxxxxxxx>
---
fs/cifs/smb2pdu.c | 7 ++++---
fs/ksmbd/smb2pdu.c | 2 +-
fs/smbfs_common/smb2pdu.h | 3 +--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 1fbb8ccf1ff6..82cd21c26c60 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1105,7 +1105,10 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
if (tcon->ses->session_flags & SMB2_SESSION_FLAG_IS_NULL)
cifs_tcon_dbg(VFS, "Unexpected null user (anonymous) auth flag sent by server\n");
- pneg_inbuf = kmalloc(sizeof(*pneg_inbuf), GFP_NOFS);
+ inbuflen = sizeof(*pneg_inbuf) +
+ sizeof(__le16) * server->vals->neg_dialect_cnt;
Good!
+
+ pneg_inbuf = kmalloc(inbuflen, GFP_NOFS);
if (!pneg_inbuf)
return -ENOMEM;
@@ -1129,8 +1132,6 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
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,
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 7da0ec466887..52a524fd2f3b 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -7392,7 +7392,7 @@ static int fsctl_validate_negotiate_info(struct ksmbd_conn *conn,
int ret = 0;
int dialect;
- if (in_buf_len < offsetof(struct validate_negotiate_info_req, Dialects) +
+ if (in_buf_len < sizeof(*neg_req) +
le16_to_cpu(neg_req->DialectCount) * sizeof(__le16))
This has the same issue as I mention in the patch 3 comment, it will
potentially dereference a non-present DialectCount field, and may
perform calculation on a negative value.
return -EINVAL;
diff --git a/fs/smbfs_common/smb2pdu.h b/fs/smbfs_common/smb2pdu.h
index 2cab413fffee..4780c72e9b3a 100644
--- a/fs/smbfs_common/smb2pdu.h
+++ b/fs/smbfs_common/smb2pdu.h
@@ -1388,13 +1388,12 @@ struct reparse_symlink_data_buffer {
} __packed;
/* See MS-FSCC 2.1.2.6 and cifspdu.h for struct reparse_posix_data */
-
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 */
+ __le16 Dialects[];
} __packed;
struct validate_negotiate_info_rsp {
