linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Roman Bolshakov <r.bolshakov@yadro.com>
To: Bart Van Assche <bvanassche@acm.org>
Cc: "Martin K . Petersen" <martin.petersen@oracle.com>,
	"James E . J . Bottomley" <jejb@linux.vnet.ibm.com>,
	<linux-scsi@vger.kernel.org>,
	Himanshu Madhani <hmadhani@marvell.com>
Subject: Re: [PATCH v2 46/58] qla2xxx: Make qlt_handle_abts_completion() more robust
Date: Mon, 12 Aug 2019 23:52:22 +0300	[thread overview]
Message-ID: <20190812205222.qmse275ofl3g52bk@SPB-NB-133.local> (raw)
In-Reply-To: <20190809030219.11296-47-bvanassche@acm.org>

On Thu, Aug 08, 2019 at 08:02:07PM -0700, Bart Van Assche wrote:
> Avoid that this function crashes if mcmd == NULL.
> 
> Cc: Himanshu Madhani <hmadhani@marvell.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/qla2xxx/qla_target.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
> index d25c3fa43601..cc0c99b5f3fb 100644
> --- a/drivers/scsi/qla2xxx/qla_target.c
> +++ b/drivers/scsi/qla2xxx/qla_target.c
> @@ -5731,7 +5731,7 @@ static void qlt_handle_abts_completion(struct scsi_qla_host *vha,
>  			    entry->error_subcode2);
>  			ha->tgt.tgt_ops->free_mcmd(mcmd);
>  		}
> -	} else {
> +	} else if (mcmd) {
>  		ha->tgt.tgt_ops->free_mcmd(mcmd);
>  	}
>  }
> -- 
> 2.22.0
> 

Thanks for working on the fix, the crash can be observed sometimes on
target shutdown.

I've been inspecting the piece of code multiple times and still don't
understand if we get mcmd == NULL only when ABTS completes successfully
or there is ABTS failure together with inability to find mcmd in the
request queue? In that case, there're two more paths that could crash.

And the second question is whether the NULL received from
qlt_ctio_to_cmd is a sign of another sporadic issue somewhere else in
the driver?

Best regards,
Roman

  reply	other threads:[~2019-08-12 21:01 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-09  3:01 [PATCH v2 00/58] qla2xxx patches for kernel v5.4 Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 01/58] qla2xxx: Make qla2x00_abort_srb() again decrease the sp reference count Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 02/58] qla2xxx: Really fix qla2xxx_eh_abort() Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 03/58] qla2xxx: Improve Linux kernel coding style conformance Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 04/58] qla2xxx: Use tabs instead of spaces for indentation Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 05/58] qla2xxx: Include the <asm/unaligned.h> header file from qla_dsd.h Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 06/58] qla2xxx: Remove an include directive from qla_mr.c Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 07/58] qla2xxx: Remove a superfluous forward declaration Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 08/58] qla2xxx: Declare the fourth ql_dump_buffer() argument const Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 09/58] qla2xxx: Change the return type of qla2x00_update_ms_fdmi_iocb() into void Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 10/58] qla2xxx: Reduce the scope of three local variables in qla2xxx_queuecommand() Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 11/58] qla2xxx: Declare qla_tgt_cmd.cdb const Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 12/58] qla2xxx: Change data_dsd into an array Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 13/58] qla2xxx: Verify locking assumptions at runtime Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 14/58] qla2xxx: Reduce the number of casts in GID list code Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 15/58] qla2xxx: Simplify qlt_lport_dump() Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 16/58] qla2xxx: Remove a superfluous pointer check Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 17/58] qla2xxx: Remove two superfluous tests Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 18/58] qla2xxx: Simplify qla24xx_abort_sp_done() Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 19/58] qla2xxx: Fix session lookup in qlt_abort_work() Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 20/58] qla2xxx: Report the firmware status code if a mailbox command fails Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 21/58] qla2xxx: Do not corrupt vha->plogi_ack_list Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 22/58] qla2xxx: Use strlcpy() instead of strncpy() Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 23/58] qla2xxx: Complain if a mailbox command times out Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 24/58] qla2xxx: Complain if parsing the version string fails Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 25/58] qla2xxx: Remove dead code Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 26/58] qla2xxx: Simplify a debug statement Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 27/58] qla2xxx: Fix qla24xx_process_bidir_cmd() Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 28/58] qla2xxx: Remove unreachable code from qla83xx_idc_lock() Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 29/58] qla2xxx: Suppress a Coveritiy complaint about integer overflow Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 30/58] qla2xxx: Suppress multiple Coverity complaint about out-of-bounds accesses Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 31/58] qla2xxx: Always check the qla2x00_wait_for_hba_online() return value Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 32/58] qla2xxx: Declare fourth qla2x00_set_model_info() argument const Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 33/58] qla2xxx: Complain if waiting for pending commands times out Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 34/58] qla2xxx: Check the PCI info string output buffer size Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 35/58] qla2xxx: Use memcpy() and strlcpy() instead of strcpy() and strncpy() Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 36/58] qla2xxx: Complain if a soft reset fails Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 37/58] qla2xxx: Introduce the be_id_t and le_id_t data types for FC src/dst IDs Bart Van Assche
2019-08-09  3:01 ` [PATCH v2 38/58] qla2xxx: Change the return type of qla24xx_read_flash_data() Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 39/58] qla2xxx: Check secondary image if reading the primary image fails Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 40/58] qla2xxx: Make it explicit that ELS pass-through IOCBs use little endian Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 41/58] qla2xxx: Set the responder mode if appropriate for ELS pass-through IOCBs Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 42/58] qla2xxx: Rework key encoding in qlt_find_host_by_d_id() Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 43/58] qla2xxx: Enable type checking for the SRB free and done callback functions Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 44/58] qla2xxx: Introduce the function qla2xxx_init_sp() Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 45/58] qla2xxx: Fix a race condition between aborting and completing a SCSI command Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 46/58] qla2xxx: Make qlt_handle_abts_completion() more robust Bart Van Assche
2019-08-12 20:52   ` Roman Bolshakov [this message]
2019-08-12 23:22     ` Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 47/58] qla2xxx: Modify NVMe include directives Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 48/58] qla2xxx: Introduce qla2xxx_get_next_handle() Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 49/58] qla2xxx: Make sure that aborted commands are freed Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 50/58] qla2xxx: Complain if sp->done() is not called from the completion path Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 51/58] qla2xxx: Let the compiler check the type of the SCSI command context pointer Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 52/58] qla2xxx: Remove superfluous sts_entry_* casts Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 53/58] qla2xxx: Report invalid mailbox status codes Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 54/58] qla2xxx: Inline the qla2x00_fcport_event_handler() function Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 55/58] qla2xxx: Introduce qla2x00_els_dcmd2_free() Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 56/58] qla2xxx: Remove two superfluous if-tests Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 57/58] qla2xxx: Simplify qla24xx_async_abort_cmd() Bart Van Assche
2019-08-09  3:02 ` [PATCH v2 58/58] qla2xxx: Fix a NULL pointer dereference Bart Van Assche
2019-08-09 14:39 ` [PATCH v2 00/58] qla2xxx patches for kernel v5.4 Himanshu Madhani
2019-08-13  1:35 ` Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190812205222.qmse275ofl3g52bk@SPB-NB-133.local \
    --to=r.bolshakov@yadro.com \
    --cc=bvanassche@acm.org \
    --cc=hmadhani@marvell.com \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).