| Message ID | 1272110272-20686-7-git-send-email-jlayton@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Apr 24, 2010 at 6:57 AM, Jeff Layton <jlayton@redhat.com> wrote:
> We can use the is_first_ses_reconnect() function to determine this.
This is a slight behavior change - we will end up issuing
another SMB negprot after reconnect (which we didn't do before).
May be harmless, but worth thinking through
On Mon, 26 Apr 2010 21:45:53 -0500 Steve French <smfrench@gmail.com> wrote: > On Sat, Apr 24, 2010 at 6:57 AM, Jeff Layton <jlayton@redhat.com> wrote: > > We can use the is_first_ses_reconnect() function to determine this. > > This is a slight behavior change - we will end up issuing > another SMB negprot after reconnect (which we didn't do before). > > May be harmless, but worth thinking through > > The comment in cifs_setup_session is misleading. I don't think there's a behavior change. cifs_reconnect does this: server->maxBuf = 0; ...and that triggers a new negprot on reconnect in the current code.
On Tue, 27 Apr 2010 07:03:20 -0400 Jeff Layton <jlayton@redhat.com> wrote: > On Mon, 26 Apr 2010 21:45:53 -0500 > Steve French <smfrench@gmail.com> wrote: > > > On Sat, Apr 24, 2010 at 6:57 AM, Jeff Layton <jlayton@redhat.com> wrote: > > > We can use the is_first_ses_reconnect() function to determine this. > > > > This is a slight behavior change - we will end up issuing > > another SMB negprot after reconnect (which we didn't do before). > > > > May be harmless, but worth thinking through > > > > > > The comment in cifs_setup_session is misleading. I don't think there's > a behavior change. cifs_reconnect does this: > > server->maxBuf = 0; > > ...and that triggers a new negprot on reconnect in the current > code. ...and I think the current behavior is correct according to the spec. The MS-CIFS doc says: "An SMB_COM_NEGOTIATE exchange MUST be completed before any other SMB messages MAY be sent to the server." ...since the server doesn't have any idea that this is a reconnect, I'd assume that some (most? all?) servers would reject other SMB's on a socket that hasn't had a negprot done on it.
On Tue, Apr 27, 2010 at 6:20 AM, Jeff Layton <jlayton@redhat.com> wrote: > On Tue, 27 Apr 2010 07:03:20 -0400 > Jeff Layton <jlayton@redhat.com> wrote: > >> On Mon, 26 Apr 2010 21:45:53 -0500 >> Steve French <smfrench@gmail.com> wrote: >> >> > On Sat, Apr 24, 2010 at 6:57 AM, Jeff Layton <jlayton@redhat.com> wrote: >> > > We can use the is_first_ses_reconnect() function to determine this. >> > >> > This is a slight behavior change - we will end up issuing >> > another SMB negprot after reconnect (which we didn't do before). >> > >> > May be harmless, but worth thinking through >> > >> > >> >> The comment in cifs_setup_session is misleading. I don't think there's >> a behavior change. cifs_reconnect does this: >> >> server->maxBuf = 0; >> >> ...and that triggers a new negprot on reconnect in the current >> code. > > ...and I think the current behavior is correct according to the spec. > The MS-CIFS doc says: > > "An SMB_COM_NEGOTIATE exchange MUST be completed before any other SMB > messages MAY be sent to the server." > > ...since the server doesn't have any idea that this is a reconnect, I'd > assume that some (most? all?) servers would reject other SMB's on a > socket that hasn't had a negprot done on it. Your explanation makes sense - merged.
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index cc622a7..8e92142 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -95,7 +95,6 @@ extern int small_smb_init_no_tc(const int smb_cmd, const int wct, struct cifsSesInfo *ses, void **request_buf); extern int CIFS_SessSetup(unsigned int xid, struct cifsSesInfo *ses, - const int stage, const struct nls_table *nls_cp); extern __u16 GetNextMid(struct TCP_Server_Info *server); extern struct timespec cifs_NTtimeToUnix(__le64 utc_nanoseconds_since_1601); diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index c2793bd..49a7358 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -2878,7 +2878,6 @@ int cifs_setup_session(unsigned int xid, struct cifsSesInfo *pSesInfo, struct nls_table *nls_info) { int rc = 0; - int first_time = 0; struct TCP_Server_Info *server = pSesInfo->server; /* what if server changes its buffer size after dropping the session? */ @@ -2899,7 +2898,6 @@ int cifs_setup_session(unsigned int xid, struct cifsSesInfo *pSesInfo, spin_unlock(&GlobalMid_Lock); } - first_time = 1; } if (rc) @@ -2913,7 +2911,7 @@ int cifs_setup_session(unsigned int xid, struct cifsSesInfo *pSesInfo, cFYI(1, "Security Mode: 0x%x Capabilities: 0x%x TimeAdjust: %d", server->secMode, server->capabilities, server->timeAdj); - rc = CIFS_SessSetup(xid, pSesInfo, first_time, nls_info); + rc = CIFS_SessSetup(xid, pSesInfo, nls_info); if (rc) { cERROR(1, "Send error in SessSetup = %d", rc); } else { diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c index da9729d..84b92df 100644 --- a/fs/cifs/sess.c +++ b/fs/cifs/sess.c @@ -35,9 +35,11 @@ extern void SMBNTencrypt(unsigned char *passwd, unsigned char *c8, unsigned char *p24); -/* 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 */ +/* + * 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 + */ static bool is_first_ses_reconnect(struct cifsSesInfo *ses) { struct list_head *tmp; @@ -447,7 +449,7 @@ static void build_ntlmssp_negotiate_blob(unsigned char *pbuffer, This function returns the length of the data in the blob */ static int build_ntlmssp_auth_blob(unsigned char *pbuffer, struct cifsSesInfo *ses, - const struct nls_table *nls_cp, int first) + const struct nls_table *nls_cp, bool first) { AUTHENTICATE_MESSAGE *sec_blob = (AUTHENTICATE_MESSAGE *)pbuffer; __u32 flags; @@ -546,7 +548,7 @@ static void setup_ntlmssp_neg_req(SESSION_SETUP_ANDX *pSMB, static int setup_ntlmssp_auth_req(SESSION_SETUP_ANDX *pSMB, struct cifsSesInfo *ses, - const struct nls_table *nls, int first_time) + const struct nls_table *nls, bool first_time) { int bloblen; @@ -559,8 +561,8 @@ static int setup_ntlmssp_auth_req(SESSION_SETUP_ANDX *pSMB, #endif int -CIFS_SessSetup(unsigned int xid, struct cifsSesInfo *ses, int first_time, - const struct nls_table *nls_cp) +CIFS_SessSetup(unsigned int xid, struct cifsSesInfo *ses, + const struct nls_table *nls_cp) { int rc = 0; int wct; @@ -577,10 +579,15 @@ CIFS_SessSetup(unsigned int xid, struct cifsSesInfo *ses, int first_time, int bytes_remaining; struct key *spnego_key = NULL; __le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */ + bool first_time; if (ses == NULL) return -EINVAL; + read_lock(&cifs_tcp_ses_lock); + first_time = is_first_ses_reconnect(ses); + read_unlock(&cifs_tcp_ses_lock); + type = ses->server->secType; cFYI(1, "sess setup type %d", type);
We can use the is_first_ses_reconnect() function to determine this. Signed-off-by: Jeff Layton <jlayton@redhat.com> --- fs/cifs/cifsproto.h | 1 - fs/cifs/connect.c | 4 +--- fs/cifs/sess.c | 21 ++++++++++++++------- 3 files changed, 15 insertions(+), 11 deletions(-)