On Sat, 19 Mar 2011 22:05:43 -0500 Steve French <smfrench@xxxxxxxxx> wrote: > On Sat, Mar 19, 2011 at 4:17 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > On Fri, 18 Mar 2011 14:51:54 -0400 > > Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > > > >> On Thu, Mar 17, 2011 at 12:17:24PM -0500, Steve French wrote: > >> > If others feel strongly about this, I don't mind changing it as > >> > Christoph suggests but > >> > - to samba people, "incrementing the rfc1001 length" would be more > >> > recognizable (than opencoding the be32_add_cpu macro), and the > >> > function name was > >> > actually Jeff's suggestion which I liked. > >> > >> I don't mind the rfc1001 length per se. What's totally braindead about > >> this is having an absolutely trivial wrapper for incrementing a field, > >> which has a different name than the field it increments. > >> > >> If you feel strongly about the rfc1001 length just rename the field. > >> > > > > FWIW, the MS-SMB doc calls this value the "Stream Protocol Length". It > > also mentions that this is actually a 24 bit field and the upper 8 bits > > are supposed to be zeroed out. > > > > Should this wrapper check for values that violate that? A little > > defensive coding in this area wouldn't hurt. > > I agree, defensive code wouldn't hurt here. We do actually check IIRC > the first byte of it (which acts as an "rfc1001" command code, where > zero is what we want for most, in cifs_demultiplex_thread). Of the > next 3 bytes, which serve as a length field, old servers had a small > maximum smb size, and thus only read two bytes, eventually some > servers allowed you to use just over 64K, and Samba has allowed more > than that for years. > cifs_demultiplex_thread checks the first byte of data that is *received*. It cannot check that what we're sending is within spec. Perhaps this wrapper should. -- 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
