________________________________________
发件人: Hillf Danton <hdanton@xxxxxxxx>
发送时间: 2021年5月24日 16:25
收件人: Zhang, Qiang
抄送: axboe@xxxxxxxxx; asml.silence@xxxxxxxxx; syzbot+6cb11ade52aa17095297@xxxxxxxxxxxxxxxxxxxxxxxxx; io-uring@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
主题: Re: [PATCH] io-wq: Fix UAF when wakeup wqe in hash waitqueue
[Please note: This e-mail is from an EXTERNAL e-mail address]
On Mon, 24 May 2021 15:18:44 +0800
> From: Zqiang <qiang.zhang@xxxxxxxxxxxxx>
>
> The syzbot report a UAF when iou-wrk accessing wqe of the hash
> waitqueue. in the case of sharing a hash waitqueue between two
> io-wq, when one of the io-wq is destroyed, all iou-wrk in this
> io-wq are awakened, all wqe belonging to this io-wq are removed
> from hash waitqueue, after that, all iou-wrk belonging to this
> io-wq begin running, suppose following scenarios, wqe[0] and wqe[1]
> belong to this io-wq, and these work has same hash value.
>
> CPU0 CPU1
> iou-wrk0(wqe[0]) iou-wrk1(wqe[1])
>
> while test_bit IO_WQ_BIT_EXIT while test_bit IO_WQ_BIT_EXIT
> io_worker_handle_work
> schedule_timeout (sleep be break by wakeup io_get_next_work
> and the IO_WQ_BIT_EXIT be set) set_bit hash
>
> test_bit IO_WQ_BIT_EXIT (return true)
> wqe->work_list (is not empty)
> io_get_next_work
> io_wq_is_hashed
> test_and_set_bit hash (is true) (hash!=-1U&&!next_hashed) true
> (there is no work other than hash work)
> io_wait_on_hash clear_bit hash
> spin_lock wq_has_sleeper (is false)
> list_empty(&wqe->wait.entry) (is true)
> __add_wait_queue (hash->wait is empty,not wakeup
> and IO_WQ_BIT_EXIT has been set,
> ........ the wqe->work_list is empty exit
> (there is no work other than hash work while loop)
> io_get_next_work will return NULL)
> return NULL (the wqe->work_list is empty
> the io_worker_handle_work is not
> called)
> io_worker_exit io_worker_exit
>
> In the above scenario, wqe may be mistakenly removing
> opportunities from the queue, this leads to when the wqe is
> released, it still in hash waitqueue. when a iou-wrk belonging
> to another io-wq access hash waitqueue will trigger UAF,
> To avoid this phenomenon, after all iou-wrk thread belonging to the
> io-wq exit, remove wqe from the hash waiqueue, at this time,
> there will be no operation to queue the wqe.
>
> Reported-by: syzbot+6cb11ade52aa17095297@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Zqiang <qiang.zhang@xxxxxxxxxxxxx>
> ---
> fs/io-wq.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/fs/io-wq.c b/fs/io-wq.c
> index 5361a9b4b47b..911a1274aabd 100644
> --- a/fs/io-wq.c
> +++ b/fs/io-wq.c
> @@ -1003,13 +1003,16 @@ static void io_wq_exit_workers(struct io_wq *wq)
> struct io_wqe *wqe = wq->wqes[node];
>
> io_wq_for_each_worker(wqe, io_wq_worker_wake, NULL);
> - spin_lock_irq(&wq->hash->wait.lock);
> - list_del_init(&wq->wqes[node]->wait.entry);
> - spin_unlock_irq(&wq->hash->wait.lock);
> }
> rcu_read_unlock();
> io_worker_ref_put(wq);
> wait_for_completion(&wq->worker_done);
> +
> + for_each_node(node) {
> + spin_lock_irq(&wq->hash->wait.lock);
> + list_del_init(&wq->wqes[node]->wait.entry);
> + spin_unlock_irq(&wq->hash->wait.lock);
> + }
> put_task_struct(wq->task);
> wq->task = NULL;
> }
> --
> 2.17.1
>Scratch scalp one inch off to work out how this is a cure given a) uaf makes
>no sense without free and b) how io workers could survive
>wait_for_completion(&wq->worker_done).
>
>If they could OTOH then this is not the pill for the leak in worker_refs.
Hello Pavel Begunkov, Hillf Danton
Sorry there is a problem with the calltrace described in my message. Please ignore this modification
Thanks
Qiang
