On Tue, Sep 21, 2010 at 6:42 PM, Jeff Layton <[email protected]> wrote: > On Tue, 21 Sep 2010 14:04:24 -0500 > Shirish Pargaonkar <[email protected]> wrote: > >> > >> > Right. I'm just not sure why we need a separate flag attached to the >> > server struct for this. Why was the "first_time" mechanism not good >> > enough here? I see no reason why that wouldn't have worked for NTLMSSP >> > too. >> >> Jeff, I will investigate but at the first glance, it looks like >> first_ses is per smb session >> and not smb connection, not sure if that would be good enough for ntlmssp. >> > > first_time is set by is_first_ses_reconnect(). The comment on that > function says: > > * Checks if this is the first smb session to be reconnected after > * the socket has been reestablished (so we know whether to use vc 0). > * Called while holding the cifs_tcp_ses_lock, so do not block > > ...which isn't entirely true, since this works even when there hasn't > been a reconnect. It just walks the list of sessions on a socket and > sees if any of them are already established (that is, need_reconnect > is false). > > So there is nominally a bug here -- sesInfoAlloc probably should set > needs_reconnect to true. But since cifs_get_smb_ses doesn't stick the > session on the server's list until after the session setup succeeds the > first time, it doesn't really cause any problems. > > -- > Jeff Layton <[email protected]> >
I think the check where it exists today first_time = is_first_ses_reconnect(ses); happens too early for ntlmssp session setup. Two session setups could still race to calculate secondary session key, calculate ciphertext, and overwrite ntlmv2 session key. calc_seckey() function would probably need to check first_time once again. calc_seckey() takes mutex lock while executing its code, is_first_ses_reconnect is called by holding, session's need_reconnect is modified under GlobalMid_Lock spin lock. Have to consider all this while considering connect and reconnect logic. The scheme in this patch, for NTLMSSP, is simple enough to follow and there will not be race during all these key calculations and it works. cifs_tcp_ses_lock as read lock. -- 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
