On Fri, Jan 28, 2011 at 1:28 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > On Fri, 28 Jan 2011 12:53:52 -0600 > Steve French <smfrench@xxxxxxxxx> wrote: > >> On Fri, Jan 28, 2011 at 6:24 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: >> > The cERROR message in checkSMB when the calculated length doesn't match >> > the RFC1001 length is incorrect in many cases. It always says that the >> > RFC1001 length is bigger than the SMB, even when it's actually the >> > reverse. >> > >> > Fix the error message to say the reverse of what it does now when the >> > SMB length goes beyond the end of the received data. Also, clarify the >> > error message when the RFC length is too big. Finally, clarify the >> > comments to show that the 512 byte limit on extra data at the end of >> > the packet is arbitrary. >> >> Do we have any data to indicate that other servers send incorrect >> rfc1001 lengths larger than 512 bytes? In eight years of test events, I >> have seen only a few responses (from Windows and NetApp IIRC) where the >> length was slightly too long. The 512 byte was chosen because it >> was large enough to pass testing to all servers we had seen, >> allowing us to workaround problematic responses from real servers >> while still limiting the probability that "randomly generated" frames >> with incorrect lengths would pass validation checks and be >> processed. >> > > I'm not aware of any. My reason for proposing the patch to remove the > +512b limitation was simply that it didn't seem necessary to impose > such a limitation. The lengths all fit within what we had received so > ignoring any extra junk tacked on the end is harmless. > >> If we have data on a byte range lock or find first response from Windows >> generating more than 512 bytes of junk then we should remove the >> check. >> >> The theory proposed to explain why we see incorrect frame lengths >> from some servers was that due to rounding or due to RFC1001 length >> being calculated before it was determined whether the frame would return >> an smb error (e.g. access control error) error - the length could be off >> by up to sizeof of one t2 infolevel Length could be off up to the sizeof of >> the fixed portion of one transact2 infolevel (all of which are less >> than 512 bytes) >> if the server e.g. decides to include one fewer findfirst entry in a response, >> or if a server fills in t2 query file info data, then gets an access error, >> leaving the data, but adjusting the smb length, leaving the incorrect >> rfc1001 len. >> In all of the theorized and observed (at test events) >> cases, the server would never be off by more than sizeof largest transact2 >> infolevel. >> >> Also note that removing this check makes it harder for new server >> manufacturers to catch serious bugs in their products as the client >> no longer will do validity checking for them. Speaking from past experience >> it is more painful to debug a server mixing up the wrong type of response >> for a request if validity checking is overly relaxed. >> >> Does anyone have data on an existing server in the field that sends >> anything off by more than >> 512 bytes (the most I remember a server being off was about 100 or so bytes) >> in frame length? >> > > I give up. The latest patch (try #2) doesn't remove any checks. It just > clarifies the cERROR printk's and the comments. Patch if fine and mergeable. I don't object to the error cleanup, but I thought you were implying that you had data on two cases where servers would still fail with such a check. -- Thanks, Steve -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
