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. -- Jeff Layton <jlayton@xxxxxxxxxx> -- 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
