| Message ID | 20211018122650.11846-1-d.bogdanov@yadro.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [RESEND] scsi: qla2xxx: fix unmap already freed sgl | expand |
> On Oct 18, 2021, at 7:26 AM, Dmitry Bogdanov <d.bogdanov@yadro.com> wrote: > > sgl is freed in the target stack in target_release_cmd_kref before > calling qlt_free_cmd, but there is an unmap of sgl in qlt_free_cmd that > causes such a panic if sgl is not yet DMA unmapped: > > NIP dma_direct_unmap_sg+0xdc/0x180 > LR dma_direct_unmap_sg+0xc8/0x180 > Call Trace: > ql_dbg_prefix+0x68/0xc0 [qla2xxx] (unreliable) > dma_unmap_sg_attrs+0x54/0xf0 > qlt_unmap_sg.part.19+0x54/0x1c0 [qla2xxx] > qlt_free_cmd+0x124/0x1d0 [qla2xxx] > tcm_qla2xxx_release_cmd+0x4c/0xa0 [tcm_qla2xxx] > target_put_sess_cmd+0x198/0x370 [target_core_mod] > transport_generic_free_cmd+0x6c/0x1b0 [target_core_mod] > tcm_qla2xxx_complete_free+0x6c/0x90 [tcm_qla2xxx] > > SGL may be left unmapped in error cases of response sending. > For instance, qlt_rdy_to_xfer maps sgl and exits when sesssion is being > deleted keeping sgl mapped. > > This patch removes use-after-free of sgl, and ensures that sgl is > unmapped for the cmd that was not send to FW. > > Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com> > --- > This patchset is intended for scsi-fix. > --- > drivers/scsi/qla2xxx/qla_target.c | 14 +++++--------- > 1 file changed, 5 insertions(+), 9 deletions(-) > > diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c > index b3478ed9b12e..7d8242c120fc 100644 > --- a/drivers/scsi/qla2xxx/qla_target.c > +++ b/drivers/scsi/qla2xxx/qla_target.c > @@ -3319,8 +3319,7 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type, > "RESET-RSP online/active/old-count/new-count = %d/%d/%d/%d.\n", > vha->flags.online, qla2x00_reset_active(vha), > cmd->reset_count, qpair->chip_reset); > - spin_unlock_irqrestore(qpair->qp_lock_ptr, flags); > - return 0; > + goto out_unmap_unlock; > } > > /* Does F/W have an IOCBs for this request */ > @@ -3445,10 +3444,6 @@ int qlt_rdy_to_xfer(struct qla_tgt_cmd *cmd) > prm.sg = NULL; > prm.req_cnt = 1; > > - /* Calculate number of entries and segments required */ > - if (qlt_pci_map_calc_cnt(&prm) != 0) > - return -EAGAIN; > - > if (!qpair->fw_started || (cmd->reset_count != qpair->chip_reset) || > (cmd->sess && cmd->sess->deleted)) { > /* > @@ -3466,6 +3461,10 @@ int qlt_rdy_to_xfer(struct qla_tgt_cmd *cmd) > return 0; > } > > + /* Calculate number of entries and segments required */ > + if (qlt_pci_map_calc_cnt(&prm) != 0) > + return -EAGAIN; > + > spin_lock_irqsave(qpair->qp_lock_ptr, flags); > /* Does F/W have an IOCBs for this request */ > res = qlt_check_reserve_free_req(qpair, prm.req_cnt); > @@ -3870,9 +3869,6 @@ void qlt_free_cmd(struct qla_tgt_cmd *cmd) > > BUG_ON(cmd->cmd_in_wq); > > - if (cmd->sg_mapped) > - qlt_unmap_sg(cmd->vha, cmd); > - > if (!cmd->q_full) > qlt_decr_num_pend_cmds(cmd->vha); > > -- > 2.25.1 > Looks Good. Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com> -- Himanshu Madhani Oracle Linux Engineering
On Mon, 18 Oct 2021 15:26:50 +0300, Dmitry Bogdanov wrote: > sgl is freed in the target stack in target_release_cmd_kref before > calling qlt_free_cmd, but there is an unmap of sgl in qlt_free_cmd that > causes such a panic if sgl is not yet DMA unmapped: > > NIP dma_direct_unmap_sg+0xdc/0x180 > LR dma_direct_unmap_sg+0xc8/0x180 > Call Trace: > ql_dbg_prefix+0x68/0xc0 [qla2xxx] (unreliable) > dma_unmap_sg_attrs+0x54/0xf0 > qlt_unmap_sg.part.19+0x54/0x1c0 [qla2xxx] > qlt_free_cmd+0x124/0x1d0 [qla2xxx] > tcm_qla2xxx_release_cmd+0x4c/0xa0 [tcm_qla2xxx] > target_put_sess_cmd+0x198/0x370 [target_core_mod] > transport_generic_free_cmd+0x6c/0x1b0 [target_core_mod] > tcm_qla2xxx_complete_free+0x6c/0x90 [tcm_qla2xxx] > > [...] Applied to 5.15/scsi-fixes, thanks! [1/1] scsi: qla2xxx: fix unmap already freed sgl https://git.kernel.org/mkp/scsi/c/4a8f71014b4d
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index b3478ed9b12e..7d8242c120fc 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -3319,8 +3319,7 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type, "RESET-RSP online/active/old-count/new-count = %d/%d/%d/%d.\n", vha->flags.online, qla2x00_reset_active(vha), cmd->reset_count, qpair->chip_reset); - spin_unlock_irqrestore(qpair->qp_lock_ptr, flags); - return 0; + goto out_unmap_unlock; } /* Does F/W have an IOCBs for this request */ @@ -3445,10 +3444,6 @@ int qlt_rdy_to_xfer(struct qla_tgt_cmd *cmd) prm.sg = NULL; prm.req_cnt = 1; - /* Calculate number of entries and segments required */ - if (qlt_pci_map_calc_cnt(&prm) != 0) - return -EAGAIN; - if (!qpair->fw_started || (cmd->reset_count != qpair->chip_reset) || (cmd->sess && cmd->sess->deleted)) { /* @@ -3466,6 +3461,10 @@ int qlt_rdy_to_xfer(struct qla_tgt_cmd *cmd) return 0; } + /* Calculate number of entries and segments required */ + if (qlt_pci_map_calc_cnt(&prm) != 0) + return -EAGAIN; + spin_lock_irqsave(qpair->qp_lock_ptr, flags); /* Does F/W have an IOCBs for this request */ res = qlt_check_reserve_free_req(qpair, prm.req_cnt); @@ -3870,9 +3869,6 @@ void qlt_free_cmd(struct qla_tgt_cmd *cmd) BUG_ON(cmd->cmd_in_wq); - if (cmd->sg_mapped) - qlt_unmap_sg(cmd->vha, cmd); - if (!cmd->q_full) qlt_decr_num_pend_cmds(cmd->vha);
sgl is freed in the target stack in target_release_cmd_kref before calling qlt_free_cmd, but there is an unmap of sgl in qlt_free_cmd that causes such a panic if sgl is not yet DMA unmapped: NIP dma_direct_unmap_sg+0xdc/0x180 LR dma_direct_unmap_sg+0xc8/0x180 Call Trace: ql_dbg_prefix+0x68/0xc0 [qla2xxx] (unreliable) dma_unmap_sg_attrs+0x54/0xf0 qlt_unmap_sg.part.19+0x54/0x1c0 [qla2xxx] qlt_free_cmd+0x124/0x1d0 [qla2xxx] tcm_qla2xxx_release_cmd+0x4c/0xa0 [tcm_qla2xxx] target_put_sess_cmd+0x198/0x370 [target_core_mod] transport_generic_free_cmd+0x6c/0x1b0 [target_core_mod] tcm_qla2xxx_complete_free+0x6c/0x90 [tcm_qla2xxx] SGL may be left unmapped in error cases of response sending. For instance, qlt_rdy_to_xfer maps sgl and exits when sesssion is being deleted keeping sgl mapped. This patch removes use-after-free of sgl, and ensures that sgl is unmapped for the cmd that was not send to FW. Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com> --- This patchset is intended for scsi-fix. --- drivers/scsi/qla2xxx/qla_target.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)