On Wed, 24 Sep 2014, Mark Knibbs wrote:

> Hi,
> 
> This follows on from something I mentioned in my recent post "[PATCH]
> storage: Don't scan target 7 for SCM USB-SCSI converters".
> 
> In drivers/usb/storage/usb.c, usb_stor_control_thread():
>       /* reject if target != 0 or if LUN is higher than
>        * the maximum known LUN
>        */
>       else if (us->srb->device->id &&
>                       !(us->fflags & US_FL_SCM_MULT_TARG)) {
>               usb_stor_dbg(us, "Bad target number (%d:%llu)\n",
>                            us->srb->device->id,
>                            us->srb->device->lun);
>               us->srb->result = DID_BAD_TARGET << 16;
>       }
> 
>       else if (us->srb->device->lun > us->max_lun) {
>               usb_stor_dbg(us, "Bad LUN (%d:%llu)\n",
>       ...
> 
> The condition (us->srb->device->id && !(us->fflags & US_FL_SCM_MULT_TARG))
> means:
>  - If not SCM_MULT_TARG, reject if us->srb->device->id is non-zero. That
>    works for the usual case.
>  - If SCM_MULT_TARG, never reject. Hmm...
> 
> Would it be better/clearer to change the condition to simply
>   (us->srb->device->id >= host->max_id)
> 
> Then the command is rejected if us->srb->device->id is too large, and works
> for the SCM_MULT_TARG case too. (host->max_id is 1 for the non-SCM_MULT_TARG
> case.) Plus the condition matches nicely with the later LUN check. Untested
> patch below.
> 
> 
> Signed-off-by: Mark Knibbs <ma...@clara.co.uk>
> 
> diff -up linux-3.17-rc6/drivers/usb/storage/usb.c.orig 
> linux-3.17-rc6/drivers/usb/storage/usb.c     
> --- linux-3.17-rc6/drivers/usb/storage/usb.c.orig     2014-09-21 
> 23:43:02.000000000 +0100
> +++ linux-3.17-rc6/drivers/usb/storage/usb.c  2014-09-24 13:09:55.000000000 
> +0100
> @@ -342,11 +342,10 @@ static int usb_stor_control_thread(void
>                       us->srb->result = DID_ERROR << 16;
>               }
>  
> -             /* reject if target != 0 or if LUN is higher than
> +             /* reject if target is invalid or if LUN is higher than
>                * the maximum known LUN
>                */
> -             else if (us->srb->device->id &&
> -                             !(us->fflags & US_FL_SCM_MULT_TARG)) {
> +             else if (us->srb->device->id >= host->max_id) {
>                       usb_stor_dbg(us, "Bad target number (%d:%llu)\n",
>                                    us->srb->device->id,
>                                    us->srb->device->lun);

That's probably okay.  As far as I know, the SCSI layer doesn't 
generate commands for targets with id >= host->max_id, so this entire 
test may be unnecessary.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to