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

[Steve French]

Not that I know of - but should be in Chris Hertel's smb document.

On Wed, Mar 31, 2010 at 11:38 AM, Suresh Jayaraman <sjayaraman at suse.de> 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?
>
>> On Wed, Mar 31, 2010 at 12:30 AM, Suresh Jayaraman <sjayaraman at suse.de> 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 <jlayton at samba.org>
>>> Signed-off-by: Suresh Jayaraman <sjayaraman at suse.de>
>>> ---
>>> �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
>



-- 
Thanks,

Steve