diff mbox series

scsi: qla2xxx: avoid crash in qlt_handle_abts_completion() if mcmd == NULL

Message ID 20191104181803.5475-1-tabraham@suse.com (mailing list archive)
State Changes Requested
Headers show
Series scsi: qla2xxx: avoid crash in qlt_handle_abts_completion() if mcmd == NULL | expand

Commit Message

Thomas Abraham Nov. 4, 2019, 6:18 p.m. UTC
qlt_ctio_to_cmd() will return a NULL mcmd if h == QLA_TGT_SKIP_HANDLE. If
the error subcodes don't match the exact codes checked a crash will occur
when calling free_mcmd on the null mcmd

Signed-off-by: Thomas Abraham <tabraham@suse.com>
---
 drivers/scsi/qla2xxx/qla_target.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Roman Bolshakov Nov. 11, 2019, 5:32 p.m. UTC | #1
Hi Thomas,

The fix for the issue was sent earlier:
https://patchwork.kernel.org/patch/11141981/

It's not important to me what fixes goes into tree but I'd like to keep
the commit message because it covers how the situation arises. Also, the
cover letter of the patch series points out another issue not covered in
either of the fixes (lack of explicit LOGO instead of BA_RJT).

Thank you,
Roman

On Mon, Nov 04, 2019 at 01:18:03PM -0500, Thomas Abraham wrote:
> qlt_ctio_to_cmd() will return a NULL mcmd if h == QLA_TGT_SKIP_HANDLE. If
> the error subcodes don't match the exact codes checked a crash will occur
> when calling free_mcmd on the null mcmd
> 
> Signed-off-by: Thomas Abraham <tabraham@suse.com>
> ---
>  drivers/scsi/qla2xxx/qla_target.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
> index a06e56224a55..611ab224662f 100644
> --- a/drivers/scsi/qla2xxx/qla_target.c
> +++ b/drivers/scsi/qla2xxx/qla_target.c
> @@ -5732,7 +5732,8 @@ static void qlt_handle_abts_completion(struct scsi_qla_host *vha,
>  			    vha->vp_idx, entry->compl_status,
>  			    entry->error_subcode1,
>  			    entry->error_subcode2);
> -			ha->tgt.tgt_ops->free_mcmd(mcmd);
> +			if (mcmd)
> +				ha->tgt.tgt_ops->free_mcmd(mcmd);
>  		}
>  	} else if (mcmd) {
>  		ha->tgt.tgt_ops->free_mcmd(mcmd);
> -- 
> 2.16.4
>
Martin K. Petersen Nov. 13, 2019, 1:41 a.m. UTC | #2
Himanshu: Ping.

Also see: https://patchwork.kernel.org/patch/11141981/

> qlt_ctio_to_cmd() will return a NULL mcmd if h == QLA_TGT_SKIP_HANDLE. If
> the error subcodes don't match the exact codes checked a crash will occur
> when calling free_mcmd on the null mcmd
>
> Signed-off-by: Thomas Abraham <tabraham@suse.com>
> ---
>  drivers/scsi/qla2xxx/qla_target.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
> index a06e56224a55..611ab224662f 100644
> --- a/drivers/scsi/qla2xxx/qla_target.c
> +++ b/drivers/scsi/qla2xxx/qla_target.c
> @@ -5732,7 +5732,8 @@ static void qlt_handle_abts_completion(struct scsi_qla_host *vha,
>  			    vha->vp_idx, entry->compl_status,
>  			    entry->error_subcode1,
>  			    entry->error_subcode2);
> -			ha->tgt.tgt_ops->free_mcmd(mcmd);
> +			if (mcmd)
> +				ha->tgt.tgt_ops->free_mcmd(mcmd);
>  		}
>  	} else if (mcmd) {
>  		ha->tgt.tgt_ops->free_mcmd(mcmd);
Lee Duncan Nov. 20, 2019, 5:08 p.m. UTC | #3
On 11/4/19 10:18 AM, Thomas Abraham wrote:
> qlt_ctio_to_cmd() will return a NULL mcmd if h == QLA_TGT_SKIP_HANDLE. If
> the error subcodes don't match the exact codes checked a crash will occur
> when calling free_mcmd on the null mcmd
> 
> Signed-off-by: Thomas Abraham <tabraham@suse.com>
> ---
>  drivers/scsi/qla2xxx/qla_target.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
> index a06e56224a55..611ab224662f 100644
> --- a/drivers/scsi/qla2xxx/qla_target.c
> +++ b/drivers/scsi/qla2xxx/qla_target.c
> @@ -5732,7 +5732,8 @@ static void qlt_handle_abts_completion(struct scsi_qla_host *vha,
>  			    vha->vp_idx, entry->compl_status,
>  			    entry->error_subcode1,
>  			    entry->error_subcode2);
> -			ha->tgt.tgt_ops->free_mcmd(mcmd);
> +			if (mcmd)
> +				ha->tgt.tgt_ops->free_mcmd(mcmd);
>  		}
>  	} else if (mcmd) {
>  		ha->tgt.tgt_ops->free_mcmd(mcmd);
> 

Reviewed-by: Lee Duncan <lduncan@suse.com>
diff mbox series

Patch

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index a06e56224a55..611ab224662f 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -5732,7 +5732,8 @@  static void qlt_handle_abts_completion(struct scsi_qla_host *vha,
 			    vha->vp_idx, entry->compl_status,
 			    entry->error_subcode1,
 			    entry->error_subcode2);
-			ha->tgt.tgt_ops->free_mcmd(mcmd);
+			if (mcmd)
+				ha->tgt.tgt_ops->free_mcmd(mcmd);
 		}
 	} else if (mcmd) {
 		ha->tgt.tgt_ops->free_mcmd(mcmd);