Re: [PATCH 2/3] 3w-xxxx: fix command completion race

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

 



On Thu, Apr 23, 2015 at 12:48 AM, Christoph Hellwig <hch@xxxxxx> wrote:
> The 3w-xxxx driver needs to tear down the dma mappings before returning
> the command to the midlayer, as there is no guarantee the sglist and
> count are valid after that point.  Also remove the dma mapping helpers
> which have another inherent race due to the request_id index.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
>  drivers/scsi/3w-xxxx.c | 42 ++++++------------------------------------
>  drivers/scsi/3w-xxxx.h |  5 -----
>  2 files changed, 6 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c
> index c75f204..2940bd7 100644
> --- a/drivers/scsi/3w-xxxx.c
> +++ b/drivers/scsi/3w-xxxx.c
> @@ -1271,32 +1271,6 @@ static int tw_initialize_device_extension(TW_Device_Extension *tw_dev)
>         return 0;
>  } /* End tw_initialize_device_extension() */
>
> -static int tw_map_scsi_sg_data(struct pci_dev *pdev, struct scsi_cmnd *cmd)
> -{
> -       int use_sg;
> -
> -       dprintk(KERN_WARNING "3w-xxxx: tw_map_scsi_sg_data()\n");
> -
> -       use_sg = scsi_dma_map(cmd);
> -       if (use_sg < 0) {
> -               printk(KERN_WARNING "3w-xxxx: tw_map_scsi_sg_data(): pci_map_sg() failed.\n");
> -               return 0;
> -       }
> -
> -       cmd->SCp.phase = TW_PHASE_SGLIST;
> -       cmd->SCp.have_data_in = use_sg;
> -
> -       return use_sg;
> -} /* End tw_map_scsi_sg_data() */
> -
> -static void tw_unmap_scsi_data(struct pci_dev *pdev, struct scsi_cmnd *cmd)
> -{
> -       dprintk(KERN_WARNING "3w-xxxx: tw_unmap_scsi_data()\n");
> -
> -       if (cmd->SCp.phase == TW_PHASE_SGLIST)
> -               scsi_dma_unmap(cmd);
> -} /* End tw_unmap_scsi_data() */
> -
>  /* This function will reset a device extension */
>  static int tw_reset_device_extension(TW_Device_Extension *tw_dev)
>  {
> @@ -1319,8 +1293,8 @@ static int tw_reset_device_extension(TW_Device_Extension *tw_dev)
>                         srb = tw_dev->srb[i];
>                         if (srb != NULL) {
>                                 srb->result = (DID_RESET << 16);
> -                               tw_dev->srb[i]->scsi_done(tw_dev->srb[i]);
> -                               tw_unmap_scsi_data(tw_dev->tw_pci_dev, tw_dev->srb[i]);
> +                               scsi_dma_unmap(srb);
> +                               srb->scsi_done(srb);
>                         }
>                 }
>         }
> @@ -1767,8 +1741,8 @@ static int tw_scsiop_read_write(TW_Device_Extension *tw_dev, int request_id)
>         command_packet->byte8.io.lba = lba;
>         command_packet->byte6.block_count = num_sectors;
>
> -       use_sg = tw_map_scsi_sg_data(tw_dev->tw_pci_dev, tw_dev->srb[request_id]);
> -       if (!use_sg)
> +       use_sg = scsi_dma_map(srb);
> +       if (use_sg <= 0)
>                 return 1;
>
>         scsi_for_each_sg(tw_dev->srb[request_id], sg, use_sg, i) {
> @@ -1955,9 +1929,6 @@ static int tw_scsi_queue_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_c
>         /* Save the scsi command for use by the ISR */
>         tw_dev->srb[request_id] = SCpnt;
>
> -       /* Initialize phase to zero */
> -       SCpnt->SCp.phase = TW_PHASE_INITIAL;
> -
>         switch (*command) {
>                 case READ_10:
>                 case READ_6:
> @@ -2185,12 +2156,11 @@ static irqreturn_t tw_interrupt(int irq, void *dev_instance)
>
>                                 /* Now complete the io */
>                                 if ((error != TW_ISR_DONT_COMPLETE)) {
> +                                       scsi_dma_unmap(tw_dev->srb[request_id]);
> +                                       tw_dev->srb[request_id]->scsi_done(tw_dev->srb[request_id]);
>                                         tw_dev->state[request_id] = TW_S_COMPLETED;
>                                         tw_state_request_finish(tw_dev, request_id);
>                                         tw_dev->posted_request_count--;
> -                                       tw_dev->srb[request_id]->scsi_done(tw_dev->srb[request_id]);
> -
> -                                       tw_unmap_scsi_data(tw_dev->tw_pci_dev, tw_dev->srb[request_id]);
>                                 }
>                         }
>
> diff --git a/drivers/scsi/3w-xxxx.h b/drivers/scsi/3w-xxxx.h
> index 29b0b84e..6f65e66 100644
> --- a/drivers/scsi/3w-xxxx.h
> +++ b/drivers/scsi/3w-xxxx.h
> @@ -195,11 +195,6 @@ static unsigned char tw_sense_table[][4] =
>  #define TW_AEN_SMART_FAIL        0x000F
>  #define TW_AEN_SBUF_FAIL         0x0024
>
> -/* Phase defines */
> -#define TW_PHASE_INITIAL 0
> -#define TW_PHASE_SINGLE 1
> -#define TW_PHASE_SGLIST 2
> -
>  /* Misc defines */
>  #define TW_ALIGNMENT_6000                    64 /* 64 bytes */
>  #define TW_ALIGNMENT_7000                     4  /* 4 bytes */
> --
> 1.9.1
>
> --
> 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

Acked-by: Adam Radford <aradford@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]