On Thu, Jan 01, 2009 at 11:19:33AM +0200, Boaz Harrosh wrote: > + buffer[1] = -1; > see below > > + /* > > + * I'm not convinced we need to try quite this hard to get VPD, but > > + * all the existing users tried this hard. > > + */ > > + result = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buffer, > > + len + 4, NULL, 30 * HZ, 3, NULL); > > + if (result) > > + return result; > > + > > + /* Sanity check that we got the page back that we asked for */ > > + if (buffer[1] != page) > > + return -EIO; > > Maybe it's just me but, if you are going to if() on a buffer byte, then you need > to make sure its initial value. Or at least document the need for a -1 at > buffer[1] buffer[] is an array of unsigned char, so -1 is 255. We could be asking for page 0xff, so your initialisation doesn't help reject bogus data. Your concern here, presumably, is that the device might transfer less than two bytes, and not return an error. I think this is quite unlikely -- and in any case, such a misbehaving device is really no different from a device which transfers complete garbage. I suppose we could pass a 'resid' to scsi_execute_req and check that at least four bytes was transferred. -- Matthew Wilcox Intel Open Source Technology Centre "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
