All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bodo Stroesser <bostroesser@gmail.com>
To: "Martin K. Petersen" <martin.petersen@oracle.com>,
	linux-scsi@vger.kernel.org, target-devel@vger.kernel.org
Cc: Bodo Stroesser <bostroesser@gmail.com>,
	Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>,
	Dan Carpenter <dan.carpenter@oracle.com>
Subject: [PATCH] scsi: target: tcmu: Avoid holding XArray lock when calling lock_page
Date: Tue, 17 May 2022 21:29:13 +0200	[thread overview]
Message-ID: <20220517192913.21405-1-bostroesser@gmail.com> (raw)

In tcmu_blocks_release, lock_page() is called to prevent a race causing
possible data corruption. Since lock_page() might sleep, calling it
while holding XArray lock is a bug.

To fix the bug switch to XArray normal API by replacing the xas_for_each
with xa_for_each_range. Since normal API does its own handling of
XArray locking, now the xas_lock and xas_unlock around the loop are
obsolete.

While keeping the code short and simple, the switch to normal API slows
down the loop slightly, which is acceptable since tcmu_blocks_release is
not relevant for performance.

Fixes: bb9b9eb0ae2e ("scsi: target: tcmu: Fix possible data corruption")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Bodo Stroesser <bostroesser@gmail.com>
---
 drivers/target/target_core_user.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index b1fd06edea59..3deaeecb712e 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1661,13 +1661,14 @@ static int tcmu_check_and_free_pending_cmd(struct tcmu_cmd *cmd)
 static u32 tcmu_blocks_release(struct tcmu_dev *udev, unsigned long first,
 				unsigned long last)
 {
-	XA_STATE(xas, &udev->data_pages, first * udev->data_pages_per_blk);
 	struct page *page;
+	unsigned long dpi;
 	u32 pages_freed = 0;
 
-	xas_lock(&xas);
-	xas_for_each(&xas, page, (last + 1) * udev->data_pages_per_blk - 1) {
-		xas_store(&xas, NULL);
+	first = first * udev->data_pages_per_blk;
+	last = (last + 1) * udev->data_pages_per_blk - 1;
+	xa_for_each_range(&udev->data_pages, dpi, page, first, last) {
+		xa_erase(&udev->data_pages, dpi);
 		/*
 		 * While reaching here there may be page faults occurring on
 		 * the to-be-released pages. A race condition may occur if
@@ -1691,7 +1692,6 @@ static u32 tcmu_blocks_release(struct tcmu_dev *udev, unsigned long first,
 		__free_page(page);
 		pages_freed++;
 	}
-	xas_unlock(&xas);
 
 	atomic_sub(pages_freed, &global_page_count);
 
-- 
2.12.3


             reply	other threads:[~2022-05-17 19:29 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-17 19:29 Bodo Stroesser [this message]
2022-05-18  1:52 ` [PATCH] scsi: target: tcmu: Avoid holding XArray lock when calling lock_page Martin K. Petersen
2022-05-20  1:09 ` 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=20220517192913.21405-1-bostroesser@gmail.com \
    --to=bostroesser@gmail.com \
    --cc=dan.carpenter@oracle.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=target-devel@vger.kernel.org \
    --cc=xiaoguang.wang@linux.alibaba.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.