linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Wagner <dwagner@suse.de>
To: linux-scsi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Nilesh Javali <njavali@marvell.com>,
	Martin Wilck <mwilck@suse.com>, Arun Easi <aeasi@marvell.com>,
	Daniel Wagner <dwagner@suse.de>
Subject: [PATCH v3 1/4] qla2xxx: Warn if done() or free() are called on an already freed srb
Date: Tue,  8 Sep 2020 10:15:13 +0200	[thread overview]
Message-ID: <20200908081516.8561-2-dwagner@suse.de> (raw)
In-Reply-To: <20200908081516.8561-1-dwagner@suse.de>

Emit a warning when ->done or ->free are called on an already freed
srb. There is a hidden use-after-free bug in the driver which corrupts
the srb memory pool which originates from the cleanup callbacks.

An extensive search didn't bring any lights on the real problem. The
initial fix was to set both pointers to NULL and try to catch invalid
accesses. But instead the memory corruption was gone and the driver
didn't crash. Since not all calling places check for NULL pointer, add
explicitly default handlers. With this we workaround the memory
corruption and add a debug help.

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 drivers/scsi/qla2xxx/qla_init.c   | 10 ++++++++++
 drivers/scsi/qla2xxx/qla_inline.h |  5 +++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 57a2d76aa691..fb7d57dc4e69 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -63,6 +63,16 @@ void qla2x00_sp_free(srb_t *sp)
 	qla2x00_rel_sp(sp);
 }
 
+void qla2xxx_rel_done_warning(srb_t *sp, int res)
+{
+	WARN_ONCE(1, "Calling done() of an already freed srb %p object\n", sp);
+}
+
+void qla2xxx_rel_free_warning(srb_t *sp)
+{
+	WARN_ONCE(1, "Calling free() of an already freed srb %p object\n", sp);
+}
+
 /* Asynchronous Login/Logout Routines -------------------------------------- */
 
 unsigned long
diff --git a/drivers/scsi/qla2xxx/qla_inline.h b/drivers/scsi/qla2xxx/qla_inline.h
index 861dc522723c..2aa6f81f87c4 100644
--- a/drivers/scsi/qla2xxx/qla_inline.h
+++ b/drivers/scsi/qla2xxx/qla_inline.h
@@ -207,10 +207,15 @@ qla2xxx_get_qpair_sp(scsi_qla_host_t *vha, struct qla_qpair *qpair,
 	return sp;
 }
 
+void qla2xxx_rel_done_warning(srb_t *sp, int res);
+void qla2xxx_rel_free_warning(srb_t *sp);
+
 static inline void
 qla2xxx_rel_qpair_sp(struct qla_qpair *qpair, srb_t *sp)
 {
 	sp->qpair = NULL;
+	sp->done = qla2xxx_rel_done_warning;
+	sp->free = qla2xxx_rel_free_warning;
 	mempool_free(sp, qpair->srb_mempool);
 	QLA_QPAIR_MARK_NOT_BUSY(qpair);
 }
-- 
2.16.4


  reply	other threads:[~2020-09-08  8:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-08  8:15 [PATCH v3 0/4] qla2xxx: A couple crash fixes Daniel Wagner
2020-09-08  8:15 ` Daniel Wagner [this message]
2020-09-08  8:15 ` [PATCH v3 2/4] qla2xxx: Simplify return value logic in qla2x00_get_sp_from_handle() Daniel Wagner
2020-09-08  8:15 ` [PATCH v3 3/4] qla2xxx: Log calling function name " Daniel Wagner
2020-09-08  8:15 ` [PATCH v3 4/4] qla2xxx: Handle incorrect entry_type entries Daniel Wagner
2020-09-09 22:38 ` [EXT] [PATCH v3 0/4] qla2xxx: A couple crash fixes Arun Easi
2020-09-10  2:12 ` Martin K. Petersen
2020-09-15 20:16 ` 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=20200908081516.8561-2-dwagner@suse.de \
    --to=dwagner@suse.de \
    --cc=aeasi@marvell.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=mwilck@suse.com \
    --cc=njavali@marvell.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).