Skip to content

Commit

Permalink
smb3: Fix out-of-bounds bug in SMB2_negotiate()
Browse files Browse the repository at this point in the history
While addressing some warnings generated by -Warray-bounds, I found this
bug that was introduced back in 2017:

  CC [M]  fs/cifs/smb2pdu.o
fs/cifs/smb2pdu.c: In function ‘SMB2_negotiate’:
fs/cifs/smb2pdu.c:822:16: warning: array subscript 1 is above array bounds
of ‘__le16[1]’ {aka ‘short unsigned int[1]’} [-Warray-bounds]
  822 |   req->Dialects[1] = cpu_to_le16(SMB30_PROT_ID);
      |   ~~~~~~~~~~~~~^~~
fs/cifs/smb2pdu.c:823:16: warning: array subscript 2 is above array bounds
of ‘__le16[1]’ {aka ‘short unsigned int[1]’} [-Warray-bounds]
  823 |   req->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
      |   ~~~~~~~~~~~~~^~~
fs/cifs/smb2pdu.c:824:16: warning: array subscript 3 is above array bounds
of ‘__le16[1]’ {aka ‘short unsigned int[1]’} [-Warray-bounds]
  824 |   req->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
      |   ~~~~~~~~~~~~~^~~
fs/cifs/smb2pdu.c:816:16: warning: array subscript 1 is above array bounds
of ‘__le16[1]’ {aka ‘short unsigned int[1]’} [-Warray-bounds]
  816 |   req->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);
      |   ~~~~~~~~~~~~~^~~

At the time, the size of array _Dialects_ was changed from 1 to 3 in struct
validate_negotiate_info_req, and then in 2019 it was changed from 3 to 4,
but those changes were never made in struct smb2_negotiate_req, which has
led to a 3 and a half years old out-of-bounds bug in function
SMB2_negotiate() (fs/cifs/smb2pdu.c).

Fix this by increasing the size of array _Dialects_ in struct
smb2_negotiate_req to 4.

Fixes: 9764c02 ("SMB3: Add support for multidialect negotiate (SMB2.1 and later)")
Fixes: d5c7076 ("smb3: add smb3.1.1 to default dialect list")
Cc: [email protected]
Signed-off-by: Gustavo A. R. Silva <[email protected]>
Signed-off-by: Steve French <[email protected]>
  • Loading branch information
GustavoARSilva authored and Steve French committed Feb 2, 2021
1 parent 1048ba8 commit 8d8d1db
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion fs/cifs/smb2pdu.h
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ struct smb2_negotiate_req {
__le32 NegotiateContextOffset; /* SMB3.1.1 only. MBZ earlier */
__le16 NegotiateContextCount; /* SMB3.1.1 only. MBZ earlier */
__le16 Reserved2;
__le16 Dialects[1]; /* One dialect (vers=) at a time for now */
__le16 Dialects[4]; /* BB expand this if autonegotiate > 4 dialects */
} __packed;

/* Dialects */
Expand Down

0 comments on commit 8d8d1db

Please sign in to comment.