On Sat, Apr 2, 2011 at 6:37 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> On Sat, 2 Apr 2011 07:34:30 -0400
> Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
>> While testing my patchset to fix asynchronous writes, I hit a bunch
>> of signature problems when testing with signing on. The problem seems
>> to be that signature checks on receive can be running at the same
>> time as a process that is sending, or even that multiple receives can
>> be checking signatures at the same time, clobbering the same data
>> structures.
>>
>> While we're at it, clean up the comments over cifs_calculate_signature
>> and add a note that the srv_mutex should be held when calling this
>> function.
>>
>> This patch seems to fix the problems for me, but I'm not clear on
>> whether it's the best approach. If it is, then this should probably
>> go to stable too.
>>
>> Cc: stable@xxxxxxxxxx
>> Cc: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
>> ---
>> fs/cifs/cifsencrypt.c | 15 +++++++++------
>> 1 files changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
>> index 5bb4b09..dfbd9f1 100644
>> --- a/fs/cifs/cifsencrypt.c
>> +++ b/fs/cifs/cifsencrypt.c
>> @@ -30,12 +30,13 @@
>> #include <linux/ctype.h>
>> #include <linux/random.h>
>>
>> -/* Calculate and return the CIFS signature based on the mac key and SMB PDU */
>> -/* the 16 byte signature must be allocated by the caller */
>> -/* Note we only use the 1st eight bytes */
>> -/* Note that the smb header signature field on input contains the
>> - sequence number before this function is called */
>> -
>> +/*
>> + * Calculate and return the CIFS signature based on the mac key and SMB PDU.
>> + * The 16 byte signature must be allocated by the caller. Note we only use the
>> + * 1st eight bytes and that the smb header signature field on input contains
>> + * the sequence number before this function is called. Also, this function
>> + * should be called with the server->srv_mutex held.
>> + */
>> static int cifs_calculate_signature(const struct smb_hdr *cifs_pdu,
>> struct TCP_Server_Info *server, char *signature)
>> {
>> @@ -209,8 +210,10 @@ int cifs_verify_signature(struct smb_hdr *cifs_pdu,
>> cpu_to_le32(expected_sequence_number);
>> cifs_pdu->Signature.Sequence.Reserved = 0;
>>
>> + mutex_lock(&server->srv_mutex);
>> rc = cifs_calculate_signature(cifs_pdu, server,
>> what_we_think_sig_should_be);
>> + mutex_unlock(&server->srv_mutex);
>>
>> if (rc)
>> return rc;
>
> Also, on a semi-related note. I consistently get this error on session
> setup with krb5i, with or without the above patch:
>
> [ 842.563693] CIFS VFS: Unexpected SMB signature
>
> I set up the kernel to dump the stack there and get this:
>
> [ 842.564981] Pid: 1530, comm: mount.cifs Not tainted 2.6.38-1.fc15.x86_64.debug #1
> [ 842.566958] Call Trace:
> [ 842.567658] [<ffffffffa00fca75>] ? cifs_check_receive+0x6e/0x84 [cifs]
> [ 842.569415] [<ffffffffa00fccf6>] ? SendReceive2+0x26b/0x2cd [cifs]
> [ 842.571079] [<ffffffffa0102128>] ? CIFS_SessSetup+0x88e/0xe36 [cifs]
> [ 842.572724] [<ffffffffa00ee4db>] ? cifs_setup_session+0x90/0x1ae [cifs]
> [ 842.574551] [<ffffffffa00ee9b3>] ? cifs_get_smb_ses+0x3ba/0x47e [cifs]
> [ 842.576257] [<ffffffffa00f0627>] ? cifs_mount+0x1bb0/0x21c5 [cifs]
> [ 842.577944] [<ffffffffa00e1b1c>] ? cifs_do_mount+0x191/0x309 [cifs]
> [ 842.579612] [<ffffffff81136544>] ? vfs_kern_mount+0xaa/0x1d7
> [ 842.581093] [<ffffffff811366d9>] ? do_kern_mount+0x4d/0xdf
> [ 842.582554] [<ffffffff8114e4bf>] ? do_mount+0x6a4/0x6f8
> [ 842.583986] [<ffffffff8114e793>] ? sys_mount+0x88/0xc2
> [ 842.585354] [<ffffffff81009b82>] ? system_call_fastpath+0x16/0x1b
>
> ...it seems like the signature checking on session setup is always
> wrong. After that though, I don't see any similar problems.
>
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
>
I think it is the first packet during session setup that cifs need not
verify signature, so type 1 packet signature check in either kerberos
within ntlmp or ntlmv2 within ntlmssp using nlmp will see such errors.
--
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
