[PATCH] nbd: fix crash during nbd_genl_disconnect

From: Mike Christie
Date: Sun Feb 09 2020 - 22:06:00 EST


If we open a nbd device, but have not done a nbd_genl_connect we will
crash in nbd_genl_disconnect when we try to flush the workqueue since it
was never allocated.

This patch moves all the flush calls to sock_shutdown and adds a check
for a valid workqueue.

Note that we flush_workqueue will do the right thing if there are no
running works so we did not need the if (i) check in nbd_start_device.
This also fixes a possible bug where you could do nbd_genl_connect, then
do a NBD_DISCONNECT and NBD_CLEAR_SOCK and the workqueue would not get
flushed.
---
drivers/block/nbd.c | 44 +++++++++++++++++---------------------------
1 file changed, 17 insertions(+), 27 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 78181908f0df..1afc70ed1f0a 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -341,9 +341,12 @@ static void nbd_complete_rq(struct request *req)
}

/*
- * Forcibly shutdown the socket causing all listeners to error
+ * Forcibly shutdown the socket causing all listeners to error. If called
+ * from a disconnect/clearing context we must make sure the recv workqueues
+ * have exited so they don't release the last nbd_config reference and try to
+ * destroy the workqueue from inside itself.
*/
-static void sock_shutdown(struct nbd_device *nbd)
+static void sock_shutdown(struct nbd_device *nbd, bool flush_work)
{
struct nbd_config *config = nbd->config;
int i;
@@ -351,7 +354,7 @@ static void sock_shutdown(struct nbd_device *nbd)
if (config->num_connections == 0)
return;
if (test_and_set_bit(NBD_RT_DISCONNECTED, &config->runtime_flags))
- return;
+ goto try_flush;

for (i = 0; i < config->num_connections; i++) {
struct nbd_sock *nsock = config->socks[i];
@@ -360,6 +363,10 @@ static void sock_shutdown(struct nbd_device *nbd)
mutex_unlock(&nsock->tx_lock);
}
dev_warn(disk_to_dev(nbd->disk), "shutting down sockets\n");
+
+try_flush:
+ if (flush_work && nbd->recv_workq)
+ flush_workqueue(nbd->recv_workq);
}

static u32 req_to_nbd_cmd_type(struct request *req)
@@ -446,7 +453,7 @@ static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req,
set_bit(NBD_RT_TIMEDOUT, &config->runtime_flags);
cmd->status = BLK_STS_IOERR;
mutex_unlock(&cmd->lock);
- sock_shutdown(nbd);
+ sock_shutdown(nbd, false);
nbd_config_put(nbd);
done:
blk_mq_complete_request(req);
@@ -910,7 +917,7 @@ static int nbd_handle_cmd(struct nbd_cmd *cmd, int index)
* for the reconnect timer don't trigger the timer again
* and instead just error out.
*/
- sock_shutdown(nbd);
+ sock_shutdown(nbd, false);
nbd_config_put(nbd);
blk_mq_start_request(req);
return -EIO;
@@ -1178,7 +1185,7 @@ static int nbd_disconnect(struct nbd_device *nbd)

static void nbd_clear_sock(struct nbd_device *nbd)
{
- sock_shutdown(nbd);
+ sock_shutdown(nbd, true);
nbd_clear_que(nbd);
nbd->task_setup = NULL;
}
@@ -1264,17 +1271,7 @@ static int nbd_start_device(struct nbd_device *nbd)

args = kzalloc(sizeof(*args), GFP_KERNEL);
if (!args) {
- sock_shutdown(nbd);
- /*
- * If num_connections is m (2 < m),
- * and NO.1 ~ NO.n(1 < n < m) kzallocs are successful.
- * But NO.(n + 1) failed. We still have n recv threads.
- * So, add flush_workqueue here to prevent recv threads
- * dropping the last config_refs and trying to destroy
- * the workqueue from inside the workqueue.
- */
- if (i)
- flush_workqueue(nbd->recv_workq);
+ sock_shutdown(nbd, true);
return -ENOMEM;
}
sk_set_memalloc(config->socks[i]->sock->sk);
@@ -1306,9 +1303,7 @@ static int nbd_start_device_ioctl(struct nbd_device *nbd, struct block_device *b
mutex_unlock(&nbd->config_lock);
ret = wait_event_interruptible(config->recv_wq,
atomic_read(&config->recv_threads) == 0);
- if (ret)
- sock_shutdown(nbd);
- flush_workqueue(nbd->recv_workq);
+ sock_shutdown(nbd, true);

mutex_lock(&nbd->config_lock);
nbd_bdev_reset(bdev);
@@ -1323,7 +1318,7 @@ static int nbd_start_device_ioctl(struct nbd_device *nbd, struct block_device *b
static void nbd_clear_sock_ioctl(struct nbd_device *nbd,
struct block_device *bdev)
{
- sock_shutdown(nbd);
+ sock_shutdown(nbd, true);
__invalidate_device(bdev, true);
nbd_bdev_reset(bdev);
if (test_and_clear_bit(NBD_RT_HAS_CONFIG_REF,
@@ -1986,12 +1981,7 @@ static void nbd_disconnect_and_put(struct nbd_device *nbd)
nbd_disconnect(nbd);
nbd_clear_sock(nbd);
mutex_unlock(&nbd->config_lock);
- /*
- * Make sure recv thread has finished, so it does not drop the last
- * config ref and try to destroy the workqueue from inside the work
- * queue.
- */
- flush_workqueue(nbd->recv_workq);
+
if (test_and_clear_bit(NBD_RT_HAS_CONFIG_REF,
&nbd->config->runtime_flags))
nbd_config_put(nbd);
--
2.20.1


--------------010007000101060400020607--