Re: [PATCH 2/2] target_core_spc: bounds check for spc_emulate_inquiry()

[Hannes Reinecke <hare@suse.de>]

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@suse.de>
>> ---
>>  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.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html