On Wed, 2011-10-19 at 08:56 -0700, Roland Dreier wrote:
> On Wed, Oct 19, 2011 at 1:42 AM, Nicholas A. Bellinger
> <nab@xxxxxxxxxxxxxxx> wrote:
> >> Any chance we can start collecting a regression test suite for these
> >> things? It should be fairly easy do that using sg3_utils.
> >
> > The problem with sg_rtpg specifically is that it uses a hardcoded
> > allocation length, which is part of the reason this was not detected
> > alot earlier..
>
> Presumably one could craft a test for this with sg_raw. But yeah it
> is a matter of someone sitting down and doing it.
>
I was thinking to just make sg_rtpg accept a --len parameter..
> With mainline code this bug would only hit if the response data
> overflowed the first sg list page, right?
>
No, transport_generic_cmd_sequencer() performs the following check and
will reject control CDBs beyond the first sg list page:
/* Let's limit control cdbs to a page, for simplicity's sake. */
if ((cmd->se_cmd_flags & SCF_SCSI_CONTROL_SG_IO_CDB) &&
size > PAGE_SIZE)
goto out_invalid_cdb_field;
> The problem I'm concerned about with the current code is that
> REPORT LUNS is limited to a one-page response, and I think
> we're going to want to support more LUNs than that...
>
I was originally a bit hesitant about merging Andy's conversion here but
in the end the complexity that was dropped by converting everything to
scatterlists is worth the pain of having to add multi-SG support to
individual CDB emulation code like REPORT LUNS.
Anyways, I think that one is straight forward enough.. The ones in
target_core_pr.c and target_core_cdb.c are going to be alot more painful
due to emulation complexity if/when they are going to need multi SGL
support than REPORT LUNs.
--nab
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
