On Fri, 2013-12-20 at 07:47 +0100, Hannes Reinecke wrote: > On 12/19/2013 11:11 PM, Nicholas A. Bellinger wrote: > > On Thu, 2013-12-19 at 14:36 +0100, Hannes Reinecke wrote: > >> Instead of using a static buffer for inquiry data we should > >> rather use the command-provided buffer and implement proper > >> bounds checking when writing to it. > >> Inquiry is by no means time-critical ... > >> > >> Signed-off-by: Hannes Reinecke <hare@xxxxxxx> > >> --- > >> drivers/target/target_core_spc.c | 391 +++++++++++++++++++++-------------- > >> include/target/target_core_backend.h | 2 +- > >> 2 files changed, 235 insertions(+), 158 deletions(-) > >> > > > > Mmmmm, so this used to be the case once upon a time, and was changed to > > the current local buffer copy + possible truncate for simplicities sake. > > > > I still favor the copy to an oversized local buffer vs. adding explicit > > size checks to every possible assignment.. > > > > How about changing the local buffer to heap memory instead, and bumping > > SE_INQUIRY_BUF to 1024 bytes..? > > > Ok. But then we should have a quick check at the start > > if (cmd->data_length > SE_INQUIRY_BUF) > len = cmd->data_length > else > len = SE_INQUIRY_BUF > > to catch oversized requests. > I don't think this is necessary, as the min_t() for the memcpy() length at the bottom of spc_emulate_inquiry() takes the lower of the two, and the rbuf is already being zero-filled. --nab -- 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