Fixed and put in cifs-2.6.git for-next pending any additional reviewing/testing
On Tue, Sep 12, 2023 at 12:34 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> Hello Steve French,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch 09ee7a3bf866: "[SMB3] send channel sequence number in SMB3
> requests after reconnects" from Aug 24, 2023, leads to the following
> Smatch complaint:
>
> fs/smb/client/smb2pdu.c:105 smb2_hdr_assemble()
> warn: variable dereferenced before check 'server' (see line 95)
>
> fs/smb/client/smb2pdu.c
> 94 shdr->Command = smb2_cmd;
> 95 if (server->dialect >= SMB30_PROT_ID) {
> ^^^^^^^^
> Unchecked dereference
>
> 96 /* After reconnect SMB3 must set ChannelSequence on subsequent reqs */
> 97 smb3_hdr = (struct smb3_hdr_req *)shdr;
> 98 /* if primary channel is not set yet, use default channel for chan sequence num */
> 99 if (SERVER_IS_CHAN(server))
> 100 smb3_hdr->ChannelSequence =
> 101 cpu_to_le16(server->primary_server->channel_sequence_num);
> 102 else
> 103 smb3_hdr->ChannelSequence = cpu_to_le16(server->channel_sequence_num);
> 104 }
> 105 if (server) {
> ^^^^^^
> The existing code assumed that server could be NULL
>
> 106 spin_lock(&server->req_lock);
> 107 /* Request up to 10 credits but don't go over the limit. */
>
> regards,
> dan carpenter
--
Thanks,
Steve
From 05d0f8f55ad60854cb706798da94276a33590445 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@xxxxxxxxxxxxx>
Date: Tue, 12 Sep 2023 14:08:36 -0500
Subject: [PATCH] smb3: move server check earlier when setting channel sequence
number
Smatch warning pointed out by Dan Carpenter:
fs/smb/client/smb2pdu.c:105 smb2_hdr_assemble()
warn: variable dereferenced before check 'server' (see line 95)
Fixes: 09ee7a3bf866 ("[SMB3] send channel sequence number in SMB3 requests after reconnects")
Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
---
fs/smb/client/smb2pdu.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 092b0087c9dc..3403188e3100 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -92,17 +92,22 @@ smb2_hdr_assemble(struct smb2_hdr *shdr, __le16 smb2_cmd,
shdr->ProtocolId = SMB2_PROTO_NUMBER;
shdr->StructureSize = cpu_to_le16(64);
shdr->Command = smb2_cmd;
- if (server->dialect >= SMB30_PROT_ID) {
- /* After reconnect SMB3 must set ChannelSequence on subsequent reqs */
- smb3_hdr = (struct smb3_hdr_req *)shdr;
- /* if primary channel is not set yet, use default channel for chan sequence num */
- if (SERVER_IS_CHAN(server))
- smb3_hdr->ChannelSequence =
- cpu_to_le16(server->primary_server->channel_sequence_num);
- else
- smb3_hdr->ChannelSequence = cpu_to_le16(server->channel_sequence_num);
- }
+
if (server) {
+ /* After reconnect SMB3 must set ChannelSequence on subsequent reqs */
+ if (server->dialect >= SMB30_PROT_ID) {
+ smb3_hdr = (struct smb3_hdr_req *)shdr;
+ /*
+ * if primary channel is not set yet, use default
+ * channel for chan sequence num
+ */
+ if (SERVER_IS_CHAN(server))
+ smb3_hdr->ChannelSequence =
+ cpu_to_le16(server->primary_server->channel_sequence_num);
+ else
+ smb3_hdr->ChannelSequence =
+ cpu_to_le16(server->channel_sequence_num);
+ }
spin_lock(&server->req_lock);
/* Request up to 10 credits but don't go over the limit. */
if (server->credits >= server->max_credits)
--
2.39.2
