Re: [PATCH v2 07/24] CIFS: Make demultiplex_thread work with SMB2 code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 22, 2012 at 9:59 AM, Pavel Shilovsky <pshilovsky@xxxxxxxxx> wrote:
> 2012/6/21 Jeff Layton <jlayton@xxxxxxxxxx>:
>> On Wed, 20 Jun 2012 18:30:47 +0400
>> Pavel Shilovsky <pshilovsky@xxxxxxxxx> wrote:
>>
>>> From: Pavel Shilovsky <piastryyy@xxxxxxxxx>
>>>
>>> Now we can process SMB2 messages: check message, get message id
>>> and wakeup awaiting routines.
>>>
>>> Signed-off-by: Pavel Shilovsky <piastryyy@xxxxxxxxx>
>>> ---
>>>  fs/cifs/Makefile     |    2 +-
>>>  fs/cifs/cifs_debug.c |    2 +-
>>>  fs/cifs/smb2misc.c   |  304 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  fs/cifs/smb2ops.c    |   37 ++++++
>>>  fs/cifs/smb2pdu.h    |   30 +++++
>>>  fs/cifs/smb2proto.h  |    2 +
>>>  6 files changed, 375 insertions(+), 2 deletions(-)
>>>  create mode 100644 fs/cifs/smb2misc.c
>>>
>>> diff --git a/fs/cifs/Makefile b/fs/cifs/Makefile
>>> index a73d7f8..b77e9ec 100644
>>> --- a/fs/cifs/Makefile
>>> +++ b/fs/cifs/Makefile
>>> @@ -16,4 +16,4 @@ cifs-$(CONFIG_CIFS_DFS_UPCALL) += dns_resolve.o cifs_dfs_ref.o
>>>
>>>  cifs-$(CONFIG_CIFS_FSCACHE) += fscache.o cache.o
>>>
>>> -cifs-$(CONFIG_CIFS_SMB2) += smb2ops.o smb2maperror.o smb2transport.o
>>> +cifs-$(CONFIG_CIFS_SMB2) += smb2ops.o smb2maperror.o smb2transport.o smb2misc.o
>>> diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
>>> index e814052..8aa8693 100644
>>> --- a/fs/cifs/cifs_debug.c
>>> +++ b/fs/cifs/cifs_debug.c
>>> @@ -65,7 +65,7 @@ void cifs_dump_detail(void *buf)
>>>       cERROR(1, "Cmd: %d Err: 0x%x Flags: 0x%x Flgs2: 0x%x Mid: %d Pid: %d",
>>>                 smb->Command, smb->Status.CifsError,
>>>                 smb->Flags, smb->Flags2, smb->Mid, smb->Pid);
>>> -     cERROR(1, "smb buf %p len %d", smb, smbCalcSize(smb));
>>> +     cERROR(1, "smb buf %p len %u", smb, smbCalcSize(smb));
>>>  #endif /* CONFIG_CIFS_DEBUG2 */
>>>  }
>>>
>>> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
>>> new file mode 100644
>>> index 0000000..03167eb
>>> --- /dev/null
>>> +++ b/fs/cifs/smb2misc.c
>>> @@ -0,0 +1,304 @@
>>> +/*
>>> + *   fs/cifs/smb2misc.c
>>> + *
>>> + *   Copyright (C) International Business Machines  Corp., 2002,2011
>>> + *                 Etersoft, 2012
>>> + *   Author(s): Steve French (sfrench@xxxxxxxxxx)
>>> + *              Pavel Shilovsky (pshilovsky@xxxxxxxxx) 2012
>>> + *
>>> + *   This library is free software; you can redistribute it and/or modify
>>> + *   it under the terms of the GNU Lesser General Public License as published
>>> + *   by the Free Software Foundation; either version 2.1 of the License, or
>>> + *   (at your option) any later version.
>>> + *
>>> + *   This library is distributed in the hope that it will be useful,
>>> + *   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
>>> + *   the GNU Lesser General Public License for more details.
>>> + *
>>> + *   You should have received a copy of the GNU Lesser General Public License
>>> + *   along with this library; if not, write to the Free Software
>>> + *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>>> + */
>>> +#include <linux/ctype.h>
>>> +#include "smb2pdu.h"
>>> +#include "cifsglob.h"
>>> +#include "cifsproto.h"
>>> +#include "smb2proto.h"
>>> +#include "cifs_debug.h"
>>> +#include "cifs_unicode.h"
>>> +#include "smb2status.h"
>>> +
>>> +static int
>>> +check_smb2_hdr(struct smb2_hdr *hdr, __u64 mid)
>>> +{
>>> +     /*
>>> +      * Make sure that this really is an SMB, that it is a response,
>>> +      * and that the message ids match
>>> +      */
>>> +     if ((*(__le32 *)hdr->ProtocolId == cpu_to_le32(0x424d53fe)) &&
>>
>>                        ... == __constant_cpu_to_le32()
>>
>>                It would also be good to turn that into a #define'ed constant and use it in the check below.
>>
>>> +         (mid == hdr->MessageId)) {
>>> +             if (hdr->Flags & SMB2_FLAGS_SERVER_TO_REDIR)
>>> +                     return 0;
>>> +             else {
>>> +                     /* only one valid case where server sends us request */
>>> +                     if (hdr->Command == SMB2_OPLOCK_BREAK)
>>> +                             return 0;
>>> +                     else
>>> +                             cERROR(1, "Received Request not response");
>>> +             }
>>> +     } else { /* bad signature or mid */
>>> +             if (*(__le32 *)hdr->ProtocolId != cpu_to_le32(0x424d53fe))
>>> +                     cERROR(1, "Bad protocol string signature header %x",
>>> +                            *(unsigned int *) hdr->ProtocolId);
>>> +             if (mid != hdr->MessageId)
>>> +                     cERROR(1, "Mids do not match");
>>> +     }
>>> +     cERROR(1, "Bad SMB detected. The Mid=%llu", hdr->MessageId);
>>> +     return 1;
>>> +}
>>> +
>>> +/*
>>> + *  The following table defines the expected "StructureSize" of SMB2 responses
>>> + *  in order by SMB2 command.  This is similar to "wct" in SMB/CIFS responses.
>>> + *
>>> + *  Note that commands are defined in smb2pdu.h in le16 but the array below is
>>> + *  indexed by command in host byte order
>>> + */
>>> +static const int smb2_rsp_struct_sizes[NUMBER_OF_SMB2_COMMANDS] = {
>>> +     /* SMB2_NEGOTIATE */ 65,
>>> +     /* SMB2_SESSION_SETUP */ 9,
>>> +     /* SMB2_LOGOFF */ 4,
>>> +     /* SMB2_TREE_CONNECT */ 16,
>>> +     /* SMB2_TREE_DISCONNECT */ 4,
>>> +     /* SMB2_CREATE */ 89,
>>> +     /* SMB2_CLOSE */ 60,
>>> +     /* SMB2_FLUSH */ 4,
>>> +     /* SMB2_READ */ 17,
>>> +     /* SMB2_WRITE */ 17,
>>> +     /* SMB2_LOCK */ 4,
>>> +     /* SMB2_IOCTL */ 49,
>>> +     /* SMB2_CANCEL */ 0, /* BB CHECK this ... not listed in documentation */
>>> +     /* SMB2_ECHO */ 4,
>>> +     /* SMB2_QUERY_DIRECTORY */ 9,
>>> +     /* SMB2_CHANGE_NOTIFY */ 9,
>>> +     /* SMB2_QUERY_INFO */ 9,
>>> +     /* SMB2_SET_INFO */ 2,
>>> +     /* SMB2_OPLOCK_BREAK */ 24 /* BB FIXME can also be 44 for lease break */
>>> +};
>>> +
>>
>> It might save a few cycles (and memory) to make the above an array of
>> __le16's and go ahead and __constant_cpu_to_le16 the values. Then you
>> wouldn't need to convert the values in the headers below.
>
> Yes, make sense.
>
>>
>>> +int
>>> +smb2_check_message(char *buf, unsigned int length)
>>> +{
>>> +     struct smb2_hdr *hdr = (struct smb2_hdr *)buf;
>>> +     struct smb2_pdu *pdu = (struct smb2_pdu *)hdr;
>>> +     __u64 mid = hdr->MessageId;
>>> +     __u32 len = get_rfc1002_length(buf);
>>> +     __u32 clc_len;  /* calculated length */
>>> +     int command;
>>> +
>>> +     /* BB disable following printk later */
>>> +     cFYI(1, "checkSMB Length: 0x%x, smb_buf_length: 0x%x", length, len);
>>> +
>>> +     /*
>>> +      * Add function to do table lookup of StructureSize by command
>>> +      * ie Validate the wct via smb2_struct_sizes table above
>>> +      */
>>> +
>>> +     if (length < 2 + sizeof(struct smb2_hdr)) {
>>> +             if ((length >= sizeof(struct smb2_hdr)) && (hdr->Status != 0)) {
>>> +                     pdu->StructureSize2 = 0;
>>> +                     /*
>>> +                      * As with SMB/CIFS, on some error cases servers may
>>> +                      * not return wct properly
>>> +                      */
>>> +                     return 0;
>>> +             } else {
>>> +                     cERROR(1, "Length less than smb header size");
>>> +             }
>>> +             return 1;
>>> +     }
>>> +     if (len > CIFSMaxBufSize + MAX_SMB2_HDR_SIZE - 4) {
>>> +             cERROR(1, "smb length greater than maximum, mid=%lld", mid);
>>> +             return 1;
>>> +     }
>>> +
>>> +     if (check_smb2_hdr(hdr, mid))
>>> +             return 1;
>>> +
>>> +     if (le16_to_cpu(hdr->StructureSize) != 64) {
>>> +             cERROR(1, "Illegal structure size %d",
>>> +                       le16_to_cpu(hdr->StructureSize));
>>> +             return 1;
>>> +     }
>>> +
>>> +     command = le16_to_cpu(hdr->Command);
>>> +     if (command >= NUMBER_OF_SMB2_COMMANDS) {
>>> +             cERROR(1, "illegal SMB2 command %d", command);
>>> +             return 1;
>>> +     }
>>> +
>>> +     if (smb2_rsp_struct_sizes[command] !=
>>> +         le16_to_cpu(pdu->StructureSize2)) {
>>> +             if ((hdr->Status == 0) ||
>>> +                 (le16_to_cpu(pdu->StructureSize2) != 9)) {
>> The 9 here should probably be a preprocessor constant:
>>
>>    #define SMB2_ERROR_STRUCTURE_SIZE2  __constant_le16_to_cpu(9)
>>
>> Then you could just compare that to a bare pdu->StructureSize2
>>
>>> +                     /* error packets have 9 byte structure size */
>>> +                     cERROR(1, "Illegal response size %d for command %d",
>>> +                                le16_to_cpu(pdu->StructureSize2), command);
>>> +                     return 1;
>>> +             }
>>> +     }
>>> +
>>> +     clc_len = smb2_calc_size(hdr);
>>> +
>>> +     if (4 + len != length) {
>>> +             cERROR(1, "Length read does not match RFC1001 length %d",
>>> +                        len);
>>> +             return 1;
>>> +     }
>>
>>
>> Does RFC1001 apply here? The error message above could be clearer.
>>
>>> +
>>> +     if (4 + len != clc_len) {
>>> +             cFYI(1, "Calculated size %d length %d mismatch for mid %lld",
>>> +                      clc_len, 4 + len, mid);
>>> +             if (clc_len == 4 + len + 1) /* BB FIXME (fix samba) */
>>> +                     return 0; /* BB workaround Samba 3 bug SessSetup rsp */
>>> +             return 1;
>>> +     }
>>> +     return 0;
>>> +}
>>> +
>>> +/*
>>> + *  The size of the variable area depends on the offset and length fields
>>> + *  located in different fields for various SMB2 responses.  SMB2 responses
>>> + *  with no variable length info, show an offset of zero for the offset field.
>>> + */
>>> +static const bool has_smb2_data_area[NUMBER_OF_SMB2_COMMANDS] = {
>>> +     /* SMB2_NEGOTIATE */ true,
>>> +     /* SMB2_SESSION_SETUP */ true,
>>> +     /* SMB2_LOGOFF */ false,
>>> +     /* SMB2_TREE_CONNECT */ false,
>>> +     /* SMB2_TREE_DISCONNECT */ false,
>>> +     /* SMB2_CREATE */ true,
>>> +     /* SMB2_CLOSE */ false,
>>> +     /* SMB2_FLUSH */ false,
>>> +     /* SMB2_READ */ true,
>>> +     /* SMB2_WRITE */ false,
>>> +     /* SMB2_LOCK */ false,
>>> +     /* SMB2_IOCTL */ true,
>>> +     /* SMB2_CANCEL */ false, /* BB CHECK this not listed in documentation */
>>> +     /* SMB2_ECHO */ false,
>>> +     /* SMB2_QUERY_DIRECTORY */ true,
>>> +     /* SMB2_CHANGE_NOTIFY */ true,
>>> +     /* SMB2_QUERY_INFO */ true,
>>> +     /* SMB2_SET_INFO */ false,
>>> +     /* SMB2_OPLOCK_BREAK */ false
>>> +};
>>> +
>>> +/*
>>> + * Returns the pointer to the beginning of the data area. Length of the data
>>> + * area and the offset to it (from the beginning of the smb are also returned.
>>> + */
>>> +static char *
>>> +smb2_get_data_area_len(int *off, int *len, struct smb2_hdr *hdr)
>>> +{
>>> +     *off = 0;
>>> +     *len = 0;
>>> +
>>> +     /* error responses do not have data area */
>>> +     if (hdr->Status &&
>>> +        (le32_to_cpu(hdr->Status) != STATUS_MORE_PROCESSING_REQUIRED) &&
>>
>> I wonder...should we also convert the STATUS_* constants to LE values?
>> That would save a few cycles on BE machines...
>
> In this case we should probably convert nterr.h too -- Steve, what do you think?

It is probably fine if it makes the cifs code a little cleaner, but I
am more concerned about optimizing the smb2.1/smb3 code than
optimizing the cifs code at this point. I don't

-- 
Thanks,

Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux