Re: [PATCH 18/19] target/iscsi: Avoid that CDB parser bugs trigger a kernel crash

["Nicholas A. Bellinger" <nab@xxxxxxxxxxxxxxx>]

On Mon, 2017-05-08 at 18:10 +0000, Bart Van Assche wrote:
> On Sun, 2017-05-07 at 15:55 -0700, Nicholas A. Bellinger wrote:
> > diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
> > index a0ad618..2cc8753 100644
> > --- a/drivers/target/target_core_sbc.c
> > +++ b/drivers/target/target_core_sbc.c
> > @@ -836,10 +836,8 @@ static sense_reason_t compare_and_write_callback(struct se_cmd *cmd, bool succes
> >   * @cmd:     (in)  structure that describes the SCSI command to be parsed.
> >   * @sectors: (out) Number of logical blocks on the storage medium that will be
> >   *           affected by the SCSI command.
> > - * @bufflen: (out) Expected length of the SCSI Data-Out buffer.
> >   */
> > -static sense_reason_t sbc_parse_verify(struct se_cmd *cmd, int *sectors,
> > -                                      u32 *bufflen)
> > +static sense_reason_t sbc_parse_verify(struct se_cmd *cmd, unsigned int *sectors)
> >  {
> >         struct se_device *dev = cmd->se_dev;
> >         u8 *cdb = cmd->t_task_cdb;
> > @@ -871,10 +869,7 @@ static sense_reason_t sbc_parse_verify(struct se_cmd *cmd, int *sectors,
> >  
> >         switch (bytchk) {
> >         case 0:
> > -               *bufflen = 0;
> > -               break;
> >         case 1:
> > -               *bufflen = sbc_get_size(cmd, *sectors);
> >                 cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
> >                 break;
> >         default:
> > @@ -967,7 +962,7 @@ static sense_reason_t sbc_parse_verify(struct se_cmd *cmd, int *sectors,
> >                 break;
> >         case WRITE_VERIFY:
> >         case WRITE_VERIFY_16:
> > -               ret = sbc_parse_verify(cmd, &sectors, &size);
> > +               ret = sbc_parse_verify(cmd, &sectors);
> >                 if (ret)
> >                         return ret;
> >                 cmd->execute_cmd = sbc_execute_rw;
> > @@ -1169,7 +1164,7 @@ static sense_reason_t sbc_parse_verify(struct se_cmd *cmd, int *sectors,
> >                 break;
> >         case VERIFY:
> >         case VERIFY_16:
> > -               ret = sbc_parse_verify(cmd, &sectors, &size);
> > +               ret = sbc_parse_verify(cmd, &sectors);
> >                 if (ret)
> >                         return ret;
> >                 cmd->execute_cmd = sbc_emulate_noop;
> 
> As I have already explained in another e-mail: the above change is completely wrong.

Nope.

If your original patch had been run through libiscsi before posting it,
you'd have already caught the regression.

So run it though libiscsi and see for yourself that it restores existing
behavior.

In any event, this is the patch I'll be merging for -rc1 to restore
existing behavior to avoid the OOPsen your change has introduced.

--
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