All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <bvanassche@acm.org>
To: "Martin K . Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org, Jaegeuk Kim <jaegeuk@kernel.org>,
	Bart Van Assche <bvanassche@acm.org>,
	Bean Huo <beanhuo@micron.com>, Avri Altman <avri.altman@wdc.com>,
	Alim Akhtar <alim.akhtar@samsung.com>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	Can Guo <cang@codeaurora.org>,
	Stanley Chu <stanley.chu@mediatek.com>,
	Asutosh Das <asutoshd@codeaurora.org>
Subject: [PATCH v3 06/18] scsi: ufs: Remove ufshcd_valid_tag()
Date: Wed, 21 Jul 2021 20:34:27 -0700	[thread overview]
Message-ID: <20210722033439.26550-7-bvanassche@acm.org> (raw)
In-Reply-To: <20210722033439.26550-1-bvanassche@acm.org>

scsi_add_host() allocates shost->can_queue tags. ufshcd_init() sets
shost->can_queue to hba->nutrs. In other words, we know that tag values
will less than hba->nutrs. Hence remove the checks that verify that
blk_get_request() returns a tag less than hba->nutrs. This check
was introduced by commit 14497328b6a6 ("scsi: ufs: verify command tag
validity").

Keep the tag >= 0 check because it helps to detect use-after-free issues.

Reviewed-by: Bean Huo <beanhuo@micron.com>
CC: Avri Altman <avri.altman@wdc.com>
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufshcd.c | 42 ++++++++++-----------------------------
 1 file changed, 10 insertions(+), 32 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 17afd1b0bd2c..ec12cd4eae03 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -252,11 +252,6 @@ static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable);
 static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
 static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba);
 
-static inline bool ufshcd_valid_tag(struct ufs_hba *hba, int tag)
-{
-	return tag >= 0 && tag < hba->nutrs;
-}
-
 static inline void ufshcd_enable_irq(struct ufs_hba *hba)
 {
 	if (!hba->is_irq_enabled) {
@@ -2700,20 +2695,12 @@ static void ufshcd_init_lrb(struct ufs_hba *hba, struct ufshcd_lrb *lrb, int i)
  */
 static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 {
+	struct ufs_hba *hba = shost_priv(host);
+	int tag = cmd->request->tag;
 	struct ufshcd_lrb *lrbp;
-	struct ufs_hba *hba;
-	int tag;
 	int err = 0;
 
-	hba = shost_priv(host);
-
-	tag = cmd->request->tag;
-	if (!ufshcd_valid_tag(hba, tag)) {
-		dev_err(hba->dev,
-			"%s: invalid command tag %d: cmd=0x%p, cmd->request=0x%p",
-			__func__, tag, cmd, cmd->request);
-		BUG();
-	}
+	WARN_ONCE(tag < 0, "Invalid tag %d\n", tag);
 
 	if (!down_read_trylock(&hba->clk_scaling_lock))
 		return SCSI_MLQUEUE_HOST_BUSY;
@@ -2967,7 +2954,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
 		goto out_unlock;
 	}
 	tag = req->tag;
-	WARN_ON_ONCE(!ufshcd_valid_tag(hba, tag));
+	WARN_ONCE(tag < 0, "Invalid tag %d\n", tag);
 	/* Set the timeout such that the SCSI error handler is not activated. */
 	req->timeout = msecs_to_jiffies(2 * timeout);
 	blk_mq_start_request(req);
@@ -6677,7 +6664,7 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
 		goto out_unlock;
 	}
 	tag = req->tag;
-	WARN_ON_ONCE(!ufshcd_valid_tag(hba, tag));
+	WARN_ONCE(tag < 0, "Invalid tag %d\n", tag);
 
 	if (unlikely(test_bit(tag, &hba->outstanding_reqs))) {
 		err = -EBUSY;
@@ -6979,24 +6966,15 @@ static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
  */
 static int ufshcd_abort(struct scsi_cmnd *cmd)
 {
-	struct Scsi_Host *host;
-	struct ufs_hba *hba;
+	struct Scsi_Host *host = cmd->device->host;
+	struct ufs_hba *hba = shost_priv(host);
+	unsigned int tag = cmd->request->tag;
+	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
 	unsigned long flags;
-	unsigned int tag;
 	int err = 0;
-	struct ufshcd_lrb *lrbp;
 	u32 reg;
 
-	host = cmd->device->host;
-	hba = shost_priv(host);
-	tag = cmd->request->tag;
-	lrbp = &hba->lrb[tag];
-	if (!ufshcd_valid_tag(hba, tag)) {
-		dev_err(hba->dev,
-			"%s: invalid command tag %d: cmd=0x%p, cmd->request=0x%p",
-			__func__, tag, cmd, cmd->request);
-		BUG();
-	}
+	WARN_ONCE(tag < 0, "Invalid tag %d\n", tag);
 
 	ufshcd_hold(hba, false);
 	reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);

  parent reply	other threads:[~2021-07-22  3:35 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-22  3:34 [PATCH v3 00/18] UFS patches for kernel v5.15 Bart Van Assche
2021-07-22  3:34 ` [PATCH v3 01/18] scsi: ufs: Fix memory corruption by ufshcd_read_desc_param() Bart Van Assche
2021-07-25 12:40   ` Avri Altman
2021-07-22  3:34 ` [PATCH v3 02/18] scsi: ufs: Reduce power management code duplication Bart Van Assche
2021-07-31 14:44   ` Stanley Chu
2021-07-22  3:34 ` [PATCH v3 03/18] scsi: ufs: Only include power management code if necessary Bart Van Assche
2021-07-22  3:34 ` [PATCH v3 04/18] scsi: ufs: Rename the second ufshcd_probe_hba() argument Bart Van Assche
2021-08-02  8:17   ` Stanley Chu
2021-07-22  3:34 ` [PATCH v3 05/18] scsi: ufs: Use DECLARE_COMPLETION_ONSTACK() where appropriate Bart Van Assche
2021-07-22  3:34 ` Bart Van Assche [this message]
2021-07-22  3:34 ` [PATCH v3 07/18] scsi: ufs: Verify UIC locking requirements at runtime Bart Van Assche
2021-07-22  3:34 ` [PATCH v3 08/18] scsi: ufs: Improve static type checking for the host controller state Bart Van Assche
2021-07-22  3:34 ` [PATCH v3 09/18] scsi: ufs: Remove several wmb() calls Bart Van Assche
2021-07-25 13:20   ` Avri Altman
2021-07-22  3:34 ` [PATCH v3 10/18] scsi: ufs: Inline ufshcd_outstanding_req_clear() Bart Van Assche
2021-07-29  7:42   ` Bean Huo
2021-07-22  3:34 ` [PATCH v3 11/18] scsi: ufs: Revert "Utilize Transfer Request List Completion Notification Register" Bart Van Assche
2021-07-29  8:03   ` Bean Huo
2021-07-29 16:10     ` Bart Van Assche
2021-07-29 16:13       ` Bart Van Assche
2021-07-29 21:14         ` Bean Huo
2021-08-02 15:24   ` Bean Huo
2021-08-03 18:49     ` Bart Van Assche
2021-07-22  3:34 ` [PATCH v3 12/18] scsi: ufs: Optimize serialization of setup_xfer_req() calls Bart Van Assche
2021-07-29  8:07   ` Bean Huo
2021-07-22  3:34 ` [PATCH v3 13/18] scsi: ufs: Optimize SCSI command processing Bart Van Assche
2021-07-29  9:12   ` Bean Huo
2021-07-29 16:11     ` Bart Van Assche
2021-08-02 12:11   ` Bean Huo
2021-07-22  3:34 ` [PATCH v3 14/18] scsi: ufs: Fix the SCSI abort handler Bart Van Assche
2021-08-02 13:15   ` Bean Huo
2021-07-22  3:34 ` [PATCH v3 15/18] scsi: ufs: Request sense data asynchronously Bart Van Assche
2021-08-02 13:16   ` Bean Huo
2021-07-22  3:34 ` [PATCH v3 16/18] scsi: ufs: Synchronize SCSI and UFS error handling Bart Van Assche
2021-08-02 14:24   ` Bean Huo
2021-08-28  9:47   ` Adrian Hunter
2021-08-29  7:17     ` Avri Altman
2021-08-29 21:33       ` Bart Van Assche
2021-08-29  9:57     ` Adrian Hunter
2021-08-29 22:18     ` Bart Van Assche
2021-08-31  7:24       ` Adrian Hunter
2021-08-31 10:04         ` Adrian Hunter
2021-08-31 17:18         ` Bart Van Assche
2021-09-01  7:42           ` Adrian Hunter
2021-09-01 20:46             ` Bart Van Assche
2021-09-02  6:02               ` Adrian Hunter
2021-07-22  3:34 ` [PATCH v3 17/18] scsi: ufs: Retry aborted SCSI commands instead of completing these successfully Bart Van Assche
2021-08-02 15:03   ` Bean Huo
2021-07-22  3:34 ` [PATCH v3 18/18] scsi: ufs: Add fault injection support Bart Van Assche
2021-08-02 15:03   ` Bean Huo
     [not found] ` <CGME20210722033524epcas2p31e41c1db6883aaa644edf23bbe8a1ca2@epcms2p4>
2021-07-28  6:48   ` [PATCH v3 06/18] scsi: ufs: Remove ufshcd_valid_tag() Daejun Park
2021-07-28 22:48     ` Bart Van Assche
     [not found]     ` <CGME20210722033524epcas2p31e41c1db6883aaa644edf23bbe8a1ca2@epcms2p7>
2021-07-29  0:26       ` Daejun Park
     [not found] ` <CGME20210722033530epcas2p4c76293e5fc5163fed3995acdd02678ce@epcms2p1>
2021-07-28  7:56   ` [PATCH v3 08/18] scsi: ufs: Improve static type checking for the host controller state Keoseong Park
     [not found] ` <CGME20210722033504epcas2p1cc3c6f61e81814004c36b89c7c9e3dd5@epcms2p5>
2021-07-29  0:56   ` [PATCH v3 01/18] scsi: ufs: Fix memory corruption by ufshcd_read_desc_param() Daejun Park
     [not found] ` <CGME20210722033510epcas2p410be4f2f387e98babeefc754a9fc1414@epcms2p2>
2021-07-29  0:56   ` [PATCH v3 02/18] scsi: ufs: Reduce power management code duplication Daejun Park
     [not found] ` <CGME20210722033513epcas2p22e4c2e6ea644992ede2739ebe381d53f@epcms2p8>
2021-07-29  0:56   ` [PATCH v3 03/18] scsi: ufs: Only include power management code if necessary Daejun Park
2021-07-31 14:48     ` Stanley Chu
     [not found] ` <CGME20210722033520epcas2p31c6f801eda7f100491c85e3f9c7d6600@epcms2p6>
2021-07-29  0:56   ` [PATCH v3 04/18] scsi: ufs: Rename the second ufshcd_probe_hba() argument Daejun Park
     [not found] ` <CGME20210722033523epcas2p22ea9a4afaeb46870638ff4429010a3c1@epcms2p7>
2021-07-29  0:57   ` [PATCH v3 05/18] scsi: ufs: Use DECLARE_COMPLETION_ONSTACK() where appropriate Daejun Park
     [not found] ` <CGME20210722033527epcas2p384eefb77dff85f5d8d59beede98b6bdc@epcms2p4>
2021-07-29  0:57   ` [PATCH v3 07/18] scsi: ufs: Verify UIC locking requirements at runtime Daejun Park
     [not found] ` <CGME20210722033531epcas2p4a4a975689ad7966d3db56dd81a7a255f@epcms2p1>
2021-07-29  0:57   ` [PATCH v3 08/18] scsi: ufs: Improve static type checking for the host controller state Daejun Park
     [not found] ` <CGME20210722033536epcas2p133eef1f5e2e5a1022ccef23e9c1035aa@epcms2p5>
2021-07-29  1:24   ` [PATCH v3 09/18] scsi: ufs: Remove several wmb() calls Daejun Park
     [not found] ` <CGME20210722033552epcas2p39f68ea806091ffa9755a25b778d70101@epcms2p2>
2021-07-29  1:25   ` [PATCH v3 12/18] scsi: ufs: Optimize serialization of setup_xfer_req() calls Daejun Park
     [not found] ` <CGME20210722033553epcas2p2818d9c1f046e8514415a72a4ddddc3db@epcms2p1>
2021-07-29  1:25   ` [PATCH v3 13/18] scsi: ufs: Optimize SCSI command processing Daejun Park
2021-08-03  2:13 ` [PATCH v3 00/18] UFS patches for kernel v5.15 Martin K. Petersen
2021-08-10  5:20 ` 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=20210722033439.26550-7-bvanassche@acm.org \
    --to=bvanassche@acm.org \
    --cc=alim.akhtar@samsung.com \
    --cc=asutoshd@codeaurora.org \
    --cc=avri.altman@wdc.com \
    --cc=beanhuo@micron.com \
    --cc=cang@codeaurora.org \
    --cc=jaegeuk@kernel.org \
    --cc=jejb@linux.ibm.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=stanley.chu@mediatek.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.