Re: cifs hang patch idea - reduce sendtimeo on socket

Re: cifs hang patch idea - reduce sendtimeo on socket

[Dave Kleikamp]

On Thu, 2006-02-16 at 21:56 +0100, Adrian Bunk wrote:
> 
> I'm a bit lost now, but I hope this information helps you in finding 
> what is going wrong?

Steve and I think we have figured this out.  In some cases, CIFSSMBRead
was returning a recently freed buffer.

CIFS: CIFSSMBRead was returning an invalid pointer in buf

Thanks to Adrian Bunk for debugging the problem

Also added a fix for 64K pages we found in loosely-related testing

Signed-off-by: Dave Kleikamp <shaggy@austin.ibm.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 38ab9f6..9d7bbd2 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -1076,13 +1076,14 @@ CIFSSMBRead(const int xid, struct cifsTc
 			cifs_small_buf_release(iov[0].iov_base);
 		else if(resp_buf_type == CIFS_LARGE_BUFFER)
 			cifs_buf_release(iov[0].iov_base);
-	} else /* return buffer to caller to free */ /* BB FIXME how do we tell caller if it is not a large buffer */ {
-		*buf = iov[0].iov_base;
+	} else if(resp_buf_type != CIFS_NO_BUFFER) {
+		/* return buffer to caller to free */ 
+		*buf = iov[0].iov_base;		
 		if(resp_buf_type == CIFS_SMALL_BUFFER)
 			*pbuf_type = CIFS_SMALL_BUFFER;
 		else if(resp_buf_type == CIFS_LARGE_BUFFER)
 			*pbuf_type = CIFS_LARGE_BUFFER;
-	}
+	} /* else no valid buffer on return - leave as null */
 
 	/* Note: On -EAGAIN error only caller can retry on handle based calls
 		since file handle passed in no longer valid */
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 0e1560a..16535b5 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1795,10 +1795,10 @@ cifs_mount(struct super_block *sb, struc
 			   conjunction with 52K kvec constraint on arch with 4K
 			   page size  */
 
-		if(cifs_sb->rsize < PAGE_CACHE_SIZE) {
-			cifs_sb->rsize = PAGE_CACHE_SIZE; 
-			/* Windows ME does this */
-			cFYI(1,("Attempt to set readsize for mount to less than one page (4096)"));
+		if(cifs_sb->rsize < 2048) {
+			cifs_sb->rsize = 2048; 
+			/* Windows ME may prefer this */
+			cFYI(1,("readsize set to minimum 2048"));
 		}
 		cifs_sb->mnt_uid = volume_info.linux_uid;
 		cifs_sb->mnt_gid = volume_info.linux_gid;

-- 
David Kleikamp
IBM Linux Technology Center

Re: cifs hang patch idea - reduce sendtimeo on socket

[Adrian Bunk]

On Wed, Feb 22, 2006 at 03:35:00PM -0600, Dave Kleikamp wrote:
> On Thu, 2006-02-16 at 21:56 +0100, Adrian Bunk wrote:
> > 
> > I'm a bit lost now, but I hope this information helps you in finding 
> > what is going wrong?
> 
> Steve and I think we have figured this out.  In some cases, CIFSSMBRead
> was returning a recently freed buffer.
>...

Thanks a lot!

In my testing, this patch applied against 2.6.14-rc4 fixed all problems 
I observed.

> David Kleikamp

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: cifs hang patch idea - reduce sendtimeo on socket

[Steve French]

Adrian Bunk wrote:

>>Steve and I think we have figured this out.  In some cases, CIFSSMBRead
>>was returning a recently freed buffer.
>>...
>>    
>>
>
>Thanks a lot!
>
>In my testing, this patch applied against 2.6.14-rc4 fixed all problems 
>I observed.
>  
>
Adrian,
Thanks for the info.  Our testing looked good too.

In the development tree for cifs we are working on reducing buffer usage 
(a good first
step is already checked in) and enabling additional important security 
features (NTLMv2 and
Kerberos for newer more secure clients and also an option to configure 
and mount for
allowing lanman and plaintext sessionsetup to allow mounts to older 
legacy servers).