On Mon, Oct 25, 2010 at 12:13 PM, Jeff Layton <jlayton@xxxxxxxxx> wrote:
> On Mon, 25 Oct 2010 11:26:53 -0500
> Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
>
>> On Mon, Oct 25, 2010 at 11:14 AM, Jeff Layton <jlayton@xxxxxxxxx> wrote:
>> > On Thu, 21 Oct 2010 06:42:55 -0500
>> > shirishpargaonkar@xxxxxxxxx wrote:
>> >
>> >> From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>> >>
>> >>
>> >> Start calculating auth response within a session. Move/Add pertinet
>> >> data structures like session key, server challenge and ntlmv2_hash in
>> >> a session structure. We should do the calculations within a session
>> >> before copying session key and response over to server data
>> >> structures because a session setup can fail.
>> >>
>> >> Only after a very first smb session succeeds, it copy/make its
>> >> session key, session key of smb connection. This key stays with
>> >> the smb connection throughout its life.
>> >> sequence_number within server is set to 0x2.
>> >>
>> >> The authentication Message Authentication Key (mak) which consists
>> >> of session key followed by client response within structure session_key
>> >> is now dynamic. Every authentication type allocates the key + response
>> >> sized memory within its session structure and later either assigns or
>> >> frees it once the client response is sent and if session's session key
>> >> becomes connetion's session key.
>> >>
>> >> ntlm/ntlmi authentication functions are rearranged. A function
>> >> named setup_ntlm_resp(), similar to setup_ntlmv2_resp(), replaces
>> >> function cifs_calculate_session_key().
>> >>
>> >> size of CIFS_SESS_KEY_SIZE is changed to 16, to reflect the byte size
>> >> of the key it holds.
>> >>
>> >>
>> >> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>> >> ---
>> >> fs/cifs/cifsencrypt.c | 113 ++++++++++++++++++++++++++++---------------------
>> >> fs/cifs/cifsglob.h | 9 +----
>> >> fs/cifs/cifspdu.h | 7 +++-
>> >> fs/cifs/cifsproto.h | 11 ++---
>> >> fs/cifs/connect.c | 18 +++++++-
>> >> fs/cifs/sess.c | 107 +++++++++++++++++-----------------------------
>> >> fs/cifs/transport.c | 6 +-
>> >> 7 files changed, 134 insertions(+), 137 deletions(-)
>> >>
>> >
>> > [...]
>> >
>> >> @@ -3178,10 +3185,11 @@ int cifs_setup_session(unsigned int xid, struct cifsSesInfo *ses,
>> >> } else {
>> >> mutex_lock(&ses->server->srv_mutex);
>> >> if (!server->session_estab) {
>> >> - memcpy(&server->session_key.data,
>> >> - &ses->auth_key.data, ses->auth_key.len);
>> >> + server->session_key.response = ses->auth_key.response;
>> >> server->session_key.len = ses->auth_key.len;
>> >> - ses->server->session_estab = true;
>> >> + server->sequence_number = 0x2;
>> >
>> > It looks like the sequence_number is protected by the Global_MidLock.
>> > It's not held here. Are there potential races with setting this value
>> > here?
>> >
>>
>> Jeff, I do not think so. This setting is happening only during the very first
>> smb session setup for that smb connection. Rest of the smb sessions
>> and subsequent smb traffic going on on that smb connection for all the
>> smb sessions ensues only after this step i.e. for signature calculations.
>> So there is no resetting sequence_number by doing this.
>> If signing is not enabled, this is not even an issue at all.
>>
>
> I'm not sure I follow...
>
> The Global_MidLock is taken in the other places where this counter is
> incremented. Why is it OK to set it here outside of that lock?
>
> Isn't it possible for two session setups to proceed in parallel? If
> not, what serializes them?
>
> --
> Jeff Layton <jlayton@xxxxxxxxx>
>
Jeff, for a brand new smb connection, if the session connections race, they
are serialized on srv_mutex cifs_setup_session. Only one session setup grabs
that lock does change server's sequene number, none of the ones that follow can
reset sequence_number because session_estab is true now.
--
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
