On Wed, Oct 13, 2010 at 7:36 AM, Jeff Layton <jlayton@xxxxxxxxx> wrote:
> On Tue, 12 Oct 2010 14:45:03 -0500
> shirishpargaonkar@xxxxxxxxx wrote:
>
>> From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>>
>> Start calculation 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 copies/makes its
>> session key, session key of smb connection. This key stays with
>> the smb connection throughout its life.
>>
>>
>> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>
>
>> bcc_ptr += CIFS_SESS_KEY_SIZE;
>> if (ses->capabilities & CAP_UNICODE) {
>> @@ -3175,6 +3176,15 @@ int cifs_setup_session(unsigned int xid, struct cifsSesInfo *ses,
>> if (rc) {
>> cERROR(1, "Send error in SessSetup = %d", rc);
>> } 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.len = ses->auth_key.len;
>> + ses->server->session_estab = true;
> ^^^^
> nit: Aren't "server" and "ses->server" the
> same? Why access them differently here?
>
>> + }
>> + mutex_unlock(&server->srv_mutex);
>> +
> ^^^^^^
> What are the rules for locking around the session_estab
> flag? Here you're holding the srv_mutex when checking
> and altering it, but in cifs_reconnect you're altering
> it without holding the mutex. Is that a potential race
> condition?
Jeff, I do not think it is a potential race. In cifs_reconnect, by the
time session_estab is set as false, all the sessions have been
invalidated and socket connection has been taken down
and restablished. Isn't that as good as new connection?
Setting session_estab under srv_mutex is mainly to prevent
newly established sessions from racing.
>
>> cFYI(1, "CIFS Session Established successfully");
>> spin_lock(&GlobalMid_Lock);
>> ses->status = CifsGood;
>
> --
> Jeff Layton <jlayton@xxxxxxxxx>
>
--
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
