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

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?

> 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 */
>>
> 
> 
> 


-- 
Suresh Jayaraman
_______________________________________________
linux-cifs-client mailing list
[email protected]
https://lists.samba.org/mailman/listinfo/linux-cifs-client