On Tue, 21 Sep 2010 21:57:44 -0500 Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote: > On Tue, Sep 21, 2010 at 6:42 PM, Jeff Layton <jlayton@xxxxxxxxx> wrote: > > On Tue, 21 Sep 2010 14:04:24 -0500 > > Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> 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 <jlayton@xxxxxxxxx> > > > > We probably ought to keep first_time or cphready per type of auth mech > since it is possible to have multiple smb sessions on the same connection using > various kinds of authentication mechanisms. > Ok, but only one of them gets to set the actual session_key. The socket is a "shared" resource of sorts and we need something that indicates which session has the "right of way" to set the session_key. The thing with traffic lights is that they only work if everyone agrees on which ones to use. ;) Your previous email sounded convincing that there is a potential race there. I think you should work through the implications of that and come up with a race-free scheme that fixes this. Don't just do it for NTLMSSP though -- fix it for all the sectypes. Cheers, -- 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
