CIFS: Add session setup/logoff capability for SMB2

DISCUSSION Dan Carpenter 2 participants 2 messages Sep 19, 2012 1 message not in the archive

Subsystems (2)

SMB client (CIFS) Kernel SMB3 server (ksmbd)

lore

Share & tools

lore

Dan Carpenter

author

2012-09-19 11:31 UTC #0 lore raw

Hello Pavel Shilovsky,

This is a semi-automatic email about new static checker warnings.

The patch 5478f9ba9a34: "CIFS: Add session setup/logoff capability for SMB2" from Dec 27, 2011, leads to the following Smatch complaint:

fs/cifs/smb2pdu.c:672 SMB2_sess_setup()
warn: variable dereferenced before check 'rsp' (see line 657)

fs/cifs/smb2pdu.c

   656			ses->Suid = rsp->hdr.SessionId;
                                    ^^^^^^^^^^^^^^^^^^

New dereference.

657 rc = decode_ntlmssp_challenge(rsp->Buffer,
658 le16_to_cpu(rsp->SecurityBufferLength), ses);
659 }
660
661 /*
662 * BB eventually add code for SPNEGO decoding of NtlmChallenge blob,
663 * but at least the raw NTLMSSP case works.
664 */
665 /*
666 * No tcon so can't do
667 * cifs_stats_inc(&tcon->stats.smb2_stats.smb2_com_fail[SMB2...]);
668 */
669 if (rc != 0)
670 goto ssetup_exit;
671

   672		if (rsp == NULL) {
                    ^^^^^^^^^^^

New check.

   673			rc = -EIO;
   674			goto ssetup_exit;

regards,
dan carpenter

A message that is not in the archive (sent off-list or not yet received): [email protected]

Pavel Shilovsky

2012-09-19 11:57 UTC #2 lore raw

2012/9/19 Dan Carpenter <[email protected]>:

Quoted 32 lines

Hello Pavel Shilovsky,
This is a semi-automatic email about new static checker warnings.
The patch 5478f9ba9a34: "CIFS: Add session setup/logoff capability for SMB2" from Dec 27, 2011, leads to the following Smatch complaint:
fs/cifs/smb2pdu.c:672 SMB2_sess_setup()
warn: variable dereferenced before check 'rsp' (see line 657)
fs/cifs/smb2pdu.c
   656                  ses->Suid = rsp->hdr.SessionId;
                                    ^^^^^^^^^^^^^^^^^^
New dereference.
   657                  rc = decode_ntlmssp_challenge(rsp->Buffer,
   658                                  le16_to_cpu(rsp->SecurityBufferLength), ses);
   659          }
   660
   661          /*
   662           * BB eventually add code for SPNEGO decoding of NtlmChallenge blob,
   663           * but at least the raw NTLMSSP case works.
   664           */
   665          /*
   666           * No tcon so can't do
   667           * cifs_stats_inc(&tcon->stats.smb2_stats.smb2_com_fail[SMB2...]);
   668           */
   669          if (rc != 0)
   670                  goto ssetup_exit;
   671
   672          if (rsp == NULL) {
                    ^^^^^^^^^^^

Quoted 8 lines

New check.

   673                  rc = -EIO;
   674                  goto ssetup_exit;

regards,
dan carpenter

Ok, it makes sense. Also I think we should remove all checks like:

if (rsp == NULL) {
        ...
}

because rsp can't be NULL in rc == 0 case.

Signature · 2 lines

Best regards,
Pavel Shilovsky.