Re: [PATCH] cifs: just ignore extra junk at the end of the SMB

[Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>]

On 01/07/2011 07:04 PM, Jeff Layton wrote:
> On Fri, 07 Jan 2011 11:22:35 +0530
> Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
> 
>> On 12/22/2010 07:09 PM, Jeff Layton wrote:
>>> If the server sends us a RFC1001 length that's larger than the SMB,
>>> then there's no reason to get our panties in a bunch and spew printk's,
>>> and there's certainly no reason just ignore the response completely like
>>> we do today. Just ignore the extra stuff on the end.
>>>
>>> This fixes:
>>>
>>>     https://bugzilla.samba.org/show_bug.cgi?id=7860
>>>
>>> Reported-by: Marcus Schopen <marcus-dLPT46B32WUjnolme5KbmQ@public.gmane.org>
>>> Tested-by: Burkhard Obergoeker <burkhard.obergoeker-VYBfkVvgaNUmRHQWxfeBDQ@public.gmane.org>
>>> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>  fs/cifs/misc.c |   25 ++++++-------------------
>>>  1 files changed, 6 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
>>> index 43f1028..b3df037 100644
>>> --- a/fs/cifs/misc.c
>>> +++ b/fs/cifs/misc.c
>>> @@ -465,26 +465,13 @@ checkSMB(struct smb_hdr *smb, __u16 mid, unsigned int length)
>>>  			if (((4 + len) & 0xFFFF) == (clc_len & 0xFFFF))
>>>  				return 0; /* bcc wrapped */
>>>  		}
>>> -		cFYI(1, "Calculated size %d vs length %d mismatch for mid %d",
>>> +
>>> +		/*
>>> +		 * We allow the server to send us an arbitrary amount of junk
>>> +		 * at the end of the SMB. Just ignore it.
>>> +		 */
>>> +		cFYI(1, "Calculated size %u vs length %u mismatch for mid %u",
>>>  				clc_len, 4 + len, smb->Mid);
>>> -		/* Windows XP can return a few bytes too much, presumably
>>> -		an illegal pad, at the end of byte range lock responses
>>> -		so we allow for that three byte pad, as long as actual
>>> -		received length is as long or longer than calculated length */
>>> -		/* We have now had to extend this more, since there is a
>>> -		case in which it needs to be bigger still to handle a
>>> -		malformed response to transact2 findfirst from WinXP when
>>> -		access denied is returned and thus bcc and wct are zero
>>> -		but server says length is 0x21 bytes too long as if the server
>>> -		forget to reset the smb rfc1001 length when it reset the
>>> -		wct and bcc to minimum size and drop the t2 parms and data */
>>> -		if ((4+len > clc_len) && (len <= clc_len + 512))
>>> -			return 0;
>>> -		else {
>>> -			cERROR(1, "RFC1001 size %d bigger than SMB for Mid=%d",
>>> -					len, smb->Mid);
>>> -			return 1;
>>> -		}
>>>  	}
>>>  	return 0;
>>>  }
>>
>> Where do we stand w.r.t this patch? Though it looks OK to me, IIRC,
>> Steve had some concerns in make the checks less stricter. Steve?
>>
>>
> 
> Ahh, I haven't see where Steve commented on this patch...

Not on this patch, but elsewhere while discussing an identical issue -
http://thread.gmane.org/gmane.network.samba.general/117198/focus=1986


-- 
Suresh Jayaraman