2006-02-22 21:35:10

by Dave Kleikamp

[permalink] [raw]
Subject: Re: cifs hang patch idea - reduce sendtimeo on socket

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 <[email protected]>
Signed-off-by: Steve French <[email protected]>

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


2006-02-26 17:51:54

by Adrian Bunk

[permalink] [raw]
Subject: Re: cifs hang patch idea - reduce sendtimeo on socket

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

2006-02-26 21:55:50

by Steve French

[permalink] [raw]
Subject: Re: cifs hang patch idea - reduce sendtimeo on socket

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).