From: bugzilla-daemon@bugzilla.kernel.org
To: linux-scsi@vger.kernel.org
Subject: [Bug 212337] scsi_debug: race at module load and module unload
Date: Thu, 18 Mar 2021 19:20:00 +0000 [thread overview]
Message-ID: <bug-212337-11613-EPKCduzKsp@https.bugzilla.kernel.org/> (raw)
In-Reply-To: <bug-212337-11613@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=212337
--- Comment #5 from Luis Chamberlain (mcgrof@kernel.org) ---
(In reply to Luis Chamberlain from comment #4)
> (In reply to d gilbert from comment #3)
> > Of course, commencing device teardown during an async scan is stress
> testing
> > that code.
>
> I'm afraid scsi_debug is filled with bug bombs bound to happen, because it
> was written without certain consideration of races now coming up as tangible
> with syfs / driver load. Namely, if you hold a lock at init and also use it
> on sysfs attributes you can easily deadlock. I discovered this issue first
> with the zram driver, and fixed the issue with try_module_get()'s on each
> driver sysfs attribute, I posted patches for that, for discussion on that
> see the post [0] [1], although discussion is mostly on the first patch, the
> patch you want to look at is the second one [1].
>
> [0] https://lkml.kernel.org/r/20210306022035.11266-2-mcgrof@kernel.org
> [1] https://lkml.kernel.org/r/20210306022035.11266-3-mcgrof@kernel.org
>
> I considered fixing scsi_debug in light of this, but given that module
> initialization is *also* calling helpers used by syfs attributes, *and* this
> is also true at module removal, I'm afraid much more care is needed here. In
> my patch to zram for the sysfs issue I mention ways to trigger the deadlock,
> if you're up for the task to fix that, it would be wonderful. But hey, these
> are separate issues. just figured you should be aware.
That was a long winded way of saying -- yes stress testing async scan +
teardown may be good, but at this point in time *these* other issues I think
are of higher priority before one can even consider stress testing async scan +
teardown.
Oh and also, someone should probably consider if this is required or not, I
have a hunch it may, but not sure:
commit 48f3c4f354ce113f45cb5adbebe0f1f06f1487f7
Author: Luis Chamberlain <mcgrof@kernel.org>
Date: Thu Mar 18 15:22:34 2021 +0000
scsi_lib: try module get before running queue
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ffe824782647..0af38f8936e4 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -22,6 +22,7 @@
#include <linux/scatterlist.h>
#include <linux/blk-mq.h>
#include <linux/ratelimit.h>
+#include <linux/module.h>
#include <asm/unaligned.h>
#include <scsi/scsi.h>
@@ -1527,6 +1528,12 @@ static int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
}
+ if (!try_module_get(host->hostt->module)) {
+ cmd->result = (DID_NO_CONNECT << 16);
+ goto done;
+
+ }
+
trace_scsi_dispatch_cmd_start(cmd);
rtn = host->hostt->queuecommand(host, cmd);
if (rtn) {
@@ -1538,6 +1545,7 @@ static int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
SCSI_LOG_MLQUEUE(3, scmd_printk(KERN_INFO, cmd,
"queuecommand : request rejected\n"));
}
+ module_put(host->hostt->module);
return rtn;
done:
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
next prev parent reply other threads:[~2021-03-18 19:20 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-18 17:09 [Bug 212337] New: scsi_debug: race at module load and module unload bugzilla-daemon
2021-03-18 17:38 ` [Bug 212337] " bugzilla-daemon
2021-03-18 18:32 ` Douglas Gilbert
2021-03-18 17:43 ` bugzilla-daemon
2021-03-18 18:42 ` bugzilla-daemon
2021-03-18 19:14 ` bugzilla-daemon
2021-03-18 21:00 ` Douglas Gilbert
2021-03-18 19:20 ` bugzilla-daemon [this message]
2021-03-18 19:22 ` bugzilla-daemon
2021-03-18 19:57 ` bugzilla-daemon
2021-03-18 21:00 ` bugzilla-daemon
2021-03-22 16:23 ` bugzilla-daemon
2021-03-23 0:37 ` Douglas Gilbert
2021-03-22 18:21 ` bugzilla-daemon
2021-03-22 18:31 ` bugzilla-daemon
2021-03-23 0:38 ` bugzilla-daemon
2021-05-04 21:18 ` bugzilla-daemon
2021-05-05 15:57 ` Douglas Gilbert
2021-05-04 21:22 ` bugzilla-daemon
2021-05-05 16:06 ` bugzilla-daemon
2021-05-07 18:25 ` bugzilla-daemon
2021-05-07 20:46 ` Douglas Gilbert
2021-05-07 20:46 ` bugzilla-daemon
2021-05-07 22:37 ` bugzilla-daemon
2021-05-07 22:46 ` bugzilla-daemon
2021-07-27 19:27 ` bugzilla-daemon
2021-07-30 20:31 ` bugzilla-daemon
2021-08-10 5:19 ` bugzilla-daemon
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=bug-212337-11613-EPKCduzKsp@https.bugzilla.kernel.org/ \
--to=bugzilla-daemon@bugzilla.kernel.org \
--cc=linux-scsi@vger.kernel.org \
/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).