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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux