On Mon, 4 Oct 2010 18:08:24 -0500 Shirish Pargaonkar <[email protected]> wrote:
> On Mon, Oct 4, 2010 at 5:41 AM, Jeff Layton <[email protected]> wrote: > > On Wed, 29 Sep 2010 09:38:48 -0500 > > [email protected] wrote: > > > >> From: Shirish Pargaonkar <[email protected]> > >> > >> > >> Fix incorrect calculation of case sensitive response length in the > >> ntlmv2 (without extended security) response. > >> > >> > >> Signed-off-by: Shirish Pargaonkar <[email protected]> > >> --- > >> fs/cifs/sess.c | 11 +++++++---- > >> 1 files changed, 7 insertions(+), 4 deletions(-) > >> > >> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c > >> index af18a50..9148fd8 100644 > >> --- a/fs/cifs/sess.c > >> +++ b/fs/cifs/sess.c > >> @@ -739,9 +739,6 @@ ssetup_ntlmssp_authenticate: > >> pSMB->req_no_secext.CaseInsensitivePasswordLength = 0; > >> /* cpu_to_le16(LM2_SESS_KEY_SIZE); */ > >> > >> - pSMB->req_no_secext.CaseSensitivePasswordLength = > >> - cpu_to_le16(sizeof(struct ntlmv2_resp)); > >> - > >> /* calculate session key */ > >> rc = setup_ntlmv2_rsp(ses, v2_sess_key, nls_cp); > >> if (rc) { > >> @@ -756,11 +753,17 @@ ssetup_ntlmssp_authenticate: > >> if (ses->tilen > 0) { > >> memcpy(bcc_ptr, ses->tiblob, ses->tilen); > >> bcc_ptr += ses->tilen; > >> + pSMB->req_no_secext.CaseSensitivePasswordLength = > >> + cpu_to_le16(sizeof(struct ntlmv2_resp) + > >> + ses->tilen); > >> /* we never did allocate ses->domainName to free */ > >> kfree(ses->tiblob); > >> ses->tiblob = NULL; > >> ses->tilen = 0; > >> - } > >> + } else > >> + pSMB->req_no_secext.CaseSensitivePasswordLength = > >> + cpu_to_le16(sizeof(struct ntlmv2_resp)); > >> + > >> if (ses->capabilities & CAP_UNICODE) { > >> if (iov[0].iov_len % 2) { > >> *bcc_ptr = 0; > > > > Looks reasonable, but I think it would be clearer to just have a single > > place that sets CaseSensitivePasswordLength. If ses->tilen > 0, then you > > add the tilen to it...but tilen will never be less than 0, so there's > > no need to have this inside that if block, right? > > > > -- > > Jeff Layton <[email protected]> > > > > I think the patch may be fine just the way it is. If the field already > has little endian value, > it may be an extra code to convert exiting value, add tilen, and store > back little endian > encoded value. That was my original thought behind this code. I'm not sure I follow your logic. I was just saying that you could move the setting of CaseSensitivePasswordLength outside of that if block altogether and eliminate the else clause. That makes the code more compact and readable. For instance: pSMB->req_no_secext.CaseSensitivePasswordLength = cpu_to_le16(sizeof(struct ntlmv2_resp) + ses->tilen); if (ses->tilen > 0) { ...etc... } The else clause just exists for when ses->tilen == 0, so there's no need for an else clause there. -- 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
