linux-kernel - [PATCH] block: fix io hung by block throttle

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210414211830.5720-1-junxiao.bi@oracle.com>
Date:   Wed, 14 Apr 2021 14:18:30 -0700
From:   Junxiao Bi <junxiao.bi@...cle.com>
To:     linux-block@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     axboe@...nel.dk
Subject: [PATCH] block: fix io hung by block throttle

There is a race bug which can cause io hung when multiple processes
run parallel in rq_qos_wait().
Let assume there were 4 processes P1/P2/P3/P4, P1/P2 were at the entry
of rq_qos_wait, and P3/P4 were waiting for io done, 2 io were inflight,
the inflight io limit was 2. See race below.

void rq_qos_wait()
{
	...
    bool has_sleeper;

	>>>> P3/P4 were in sleeper list, has_sleeper was true for both P1 and P2.
    has_sleeper = wq_has_sleeper(&rqw->wait);
    if (!has_sleeper && acquire_inflight_cb(rqw, private_data))
        return;

	>>>> 2 inflight io done, P3/P4 were waken up to issue 2 new io.
	>>>> 2 new io done, no inflight io.

	>>>> P1/P2 were added to the sleeper list, 2 entry in the list
    prepare_to_wait_exclusive(&rqw->wait, &data.wq, TASK_UNINTERRUPTIBLE);

	>>>> P1/P2 were in the sleeper list, has_sleeper was true for P1/P2.
    has_sleeper = !wq_has_single_sleeper(&rqw->wait);
    do {
        /* The memory barrier in set_task_state saves us here. */
        if (data.got_token)
            break;
        if (!has_sleeper && acquire_inflight_cb(rqw, private_data)) {
            finish_wait(&rqw->wait, &data.wq);

            /*
             * We raced with wbt_wake_function() getting a token,
             * which means we now have two. Put our local token
             * and wake anyone else potentially waiting for one.
             */
            smp_rmb();
            if (data.got_token)
                cleanup_cb(rqw, private_data);
            break;
        }

	>>>> P1/P2 hung here forever. New io requests will also hung here.
        io_schedule();
        has_sleeper = true;
        set_current_state(TASK_UNINTERRUPTIBLE);
    } while (1);
    finish_wait(&rqw->wait, &data.wq);
}

Cc: stable@...r.kernel.org
Signed-off-by: Junxiao Bi <junxiao.bi@...cle.com>
---
 block/blk-rq-qos.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
index 656460636ad3..04d888c99bc0 100644
--- a/block/blk-rq-qos.c
+++ b/block/blk-rq-qos.c
@@ -260,19 +260,17 @@ void rq_qos_wait(struct rq_wait *rqw, void *private_data,
 		.cb = acquire_inflight_cb,
 		.private_data = private_data,
 	};
-	bool has_sleeper;
 
-	has_sleeper = wq_has_sleeper(&rqw->wait);
-	if (!has_sleeper && acquire_inflight_cb(rqw, private_data))
+	if (!wq_has_sleeper(&rqw->wait)
+		&& acquire_inflight_cb(rqw, private_data))
 		return;
 
 	prepare_to_wait_exclusive(&rqw->wait, &data.wq, TASK_UNINTERRUPTIBLE);
-	has_sleeper = !wq_has_single_sleeper(&rqw->wait);
 	do {
 		/* The memory barrier in set_task_state saves us here. */
 		if (data.got_token)
 			break;
-		if (!has_sleeper && acquire_inflight_cb(rqw, private_data)) {
+		if (acquire_inflight_cb(rqw, private_data)) {
 			finish_wait(&rqw->wait, &data.wq);
 
 			/*
@@ -286,7 +284,6 @@ void rq_qos_wait(struct rq_wait *rqw, void *private_data,
 			break;
 		}
 		io_schedule();
-		has_sleeper = true;
 		set_current_state(TASK_UNINTERRUPTIBLE);
 	} while (1);
 	finish_wait(&rqw->wait, &data.wq);
-- 
2.24.3 (Apple Git-128)