Re: [PATCH] cifs: Fix a kernel BUG with remote OS/2 server (try #3)

On Wed, 31 Mar 2010 23:08:56 +0530
Suresh Jayaraman <[email protected]> wrote:

> On 03/31/2010 06:52 PM, Steve French wrote:
> > I also like the change to this patch slightly more.  This and previous
> > patch merged.  Will send upstream request after a few days.
> 
> Thanks, but just out of curiosity - Is there a reference that explains
> how CountHigh could/could not be used by the server and the client?
> 

+1

While the fix you have here seems like it'll prevent most problems, it
sure would be nice to have a flag or something to key off of that
states that the CountHigh field should be trusted.

> > On Wed, Mar 31, 2010 at 12:30 AM, Suresh Jayaraman <[email protected]> wrote:
> >> (Please consider for -stable once reviewed and accepted).
> >>
> >> While chasing a bug report involving a OS/2 server, I noticed the server sets
> >> pSMBr->CountHigh to a incorrect value even in case of normal writes. This
> >> results in 'nbytes' being computed wrongly and triggers a kernel BUG at
> >> mm/filemap.c.
> >>
> >> void iov_iter_advance(struct iov_iter *i, size_t bytes)
> >> {
> >> � � � �BUG_ON(i->count < bytes); � �<--- BUG here
> >>
> >> Why the server is setting 'CountHigh' is not clear but only does so after
> >> writing 64k bytes. Though this looks like the server bug, the client side
> >> crash may not be acceptable.
> >>
> >> The workaround is to mask off high 16 bits if the number of bytes written as
> >> returned by the server is greater than the bytes requested by the client as
> >> suggested by Jeff Layton.
> >>
> >> Cc: Jeff Layton <[email protected]>
> >> Signed-off-by: Suresh Jayaraman <[email protected]>
> >> ---
> >> �fs/cifs/cifssmb.c | � 16 ++++++++++++++++
> >> �1 files changed, 16 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> >> index 7cc7f83..7d8ada8 100644
> >> --- a/fs/cifs/cifssmb.c
> >> +++ b/fs/cifs/cifssmb.c
> >> @@ -1517,6 +1517,14 @@ CIFSSMBWrite(const int xid, struct cifsTconInfo *tcon,
> >> � � � � � � � �*nbytes = le16_to_cpu(pSMBr->CountHigh);
> >> � � � � � � � �*nbytes = (*nbytes) << 16;
> >> � � � � � � � �*nbytes += le16_to_cpu(pSMBr->Count);
> >> +
> >> + � � � � � � � /*
> >> + � � � � � � � �* Mask off high 16 bits when bytes written as returned by the
> >> + � � � � � � � �* server is greater than bytes requested by the client. Some
> >> + � � � � � � � �* OS/2 servers are known to set incorrect CountHigh values.
> >> + � � � � � � � �*/
> >> + � � � � � � � if (*nbytes > count)
> >> + � � � � � � � � � � � *nbytes &= 0xFFFF;
> >> � � � �}
> >>
> >> � � � �cifs_buf_release(pSMB);
> >> @@ -1605,6 +1613,14 @@ CIFSSMBWrite2(const int xid, struct cifsTconInfo *tcon,
> >> � � � � � � � �*nbytes = le16_to_cpu(pSMBr->CountHigh);
> >> � � � � � � � �*nbytes = (*nbytes) << 16;
> >> � � � � � � � �*nbytes += le16_to_cpu(pSMBr->Count);
> >> +
> >> + � � � � � � � /*
> >> + � � � � � � � �* Mask off high 16 bits when bytes written as returned by the
> >> + � � � � � � � �* server is greater than bytes requested by the client. OS/2
> >> + � � � � � � � �* servers are known to set incorrect CountHigh values.
> >> + � � � � � � � �*/
> >> + � � � � � � � if (*nbytes > count)
> >> + � � � � � � � � � � � *nbytes &= 0xFFFF;
> >> � � � �}
> >>
> >> �/* � � cifs_small_buf_release(pSMB); */ /* Freed earlier now in SendReceive2 */
> >>
> > 
> > 
> > 
> 
> 


-- 
Jeff Layton <[email protected]>
_______________________________________________
linux-cifs-client mailing list
[email protected]
https://lists.samba.org/mailman/listinfo/linux-cifs-client