On Wed, Oct 13, 2010 at 7:36 AM, Jeff Layton <[email protected]> wrote: > On Tue, 12 Oct 2010 14:45:03 -0500 > [email protected] wrote: > >> From: Shirish Pargaonkar <[email protected]> >> >> 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 <[email protected]> > > >> 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 <[email protected]> > -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
