From: Al Viro <viro@zeniv.linux.org.uk>
To: linux-block@vger.kernel.org
Cc: linux-scsi@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH][RFC] fix a race between bsg_open() and bsg_unregister_queue()
Date: Sun, 20 Nov 2022 03:14:25 +0000 [thread overview]
Message-ID: <Y3mbkZCESLLRMQNq@ZenIV> (raw)
Consider the following scenario:
task A: open() on /dev/bsg/<something>
calls chrdev_open()
finds and grabs a reference to bsg_device.cdev in inode->i_cdev
refcount on that cdev is 2 now (1 from creation + 1 we'd just grabbed)
calls bsg_open().
fetches to_bsg_device(inode)->queue - that would be ->queue in the
same bsg_device instance.
gets preempted away and loses CPU before it gets to calling blk_get_queue().
task B: calls bsg_unregister_queue() on the same queue, which calls
cdev_device_del(), which makes cdev impossible to look up and
drops the reference to that cdev; refcount is 1 now, so nothing gets
freed yet.
caller of bsg_unregister_queue() proceeds to destroy the queue and
free it, allowing reuse of memory that used to contain it.
task A: regains CPU
calls blk_get_queue() on something that no longer points to
a request_queue instance. In particular, "dying" flag is no longer
guaranteed to be there, so we proceed to increment what we think is
a queue refcount, corrupting whatever lives in that memory now.
Usually we'll end up with memory not reused yet, and blk_get_queue() will
fail without buggering anything up. Not guaranteed, though...
AFAICS, the fact that request_queue freeing is RCU-delayed means that
it can be fixed by the following:
* mark bsg_device on bsg_unregister_queue() as goner
* have bsg_open() do rcu_read_lock(), then check that flag and do
blk_get_queue() only if the flag hadn't been set yet. If we did not observe
the flag after rcu_read_lock(), we know that queue have been freed yet
- RCU delay couldn't have run out.
Comments?
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/block/bsg.c b/block/bsg.c
index 2ab1351eb082..643641087691 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -28,6 +28,7 @@ struct bsg_device {
unsigned int timeout;
unsigned int reserved_size;
bsg_sg_io_fn *sg_io_fn;
+ bool goner;
};
static inline struct bsg_device *to_bsg_device(struct inode *inode)
@@ -71,9 +72,14 @@ static int bsg_sg_io(struct bsg_device *bd, fmode_t mode, void __user *uarg)
static int bsg_open(struct inode *inode, struct file *file)
{
- if (!blk_get_queue(to_bsg_device(inode)->queue))
- return -ENXIO;
- return 0;
+ struct bsg_device *bd = to_bsg_device(inode);
+ int err = 0;
+
+ rcu_read_lock();
+ if (bd->goner || !blk_get_queue(bd->queue))
+ err = -ENXIO;
+ rcu_read_unlock();
+ return err;
}
static int bsg_release(struct inode *inode, struct file *file)
@@ -175,6 +181,7 @@ static void bsg_device_release(struct device *dev)
void bsg_unregister_queue(struct bsg_device *bd)
{
+ bd->goner = true;
if (bd->queue->kobj.sd)
sysfs_remove_link(&bd->queue->kobj, "bsg");
cdev_device_del(&bd->cdev, &bd->device);
next reply other threads:[~2022-11-20 3:14 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-20 3:14 Al Viro [this message]
2022-11-21 8:04 ` [PATCH][RFC] fix a race between bsg_open() and bsg_unregister_queue() Christoph Hellwig
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=Y3mbkZCESLLRMQNq@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=axboe@kernel.dk \
--cc=fujita.tomonori@lab.ntt.co.jp \
--cc=linux-block@vger.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 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.