public inbox for [email protected]
 help / color / mirror / Atom feed
From: "Zhang, Qiang" <[email protected]>
To: Hillf Danton <[email protected]>,
	"[email protected]" <[email protected]>,
	"[email protected]" <[email protected]>
Cc: "[email protected]" 
	<[email protected]>,
	"[email protected]" <[email protected]>,
	"[email protected]" <[email protected]>
Subject: 回复: [PATCH] io-wq: Fix UAF when wakeup wqe in hash waitqueue
Date: Mon, 24 May 2021 09:19:14 +0000	[thread overview]
Message-ID: <DM6PR11MB4202B442C4C27740B6EE2D64FF269@DM6PR11MB4202.namprd11.prod.outlook.com> (raw)
In-Reply-To: <[email protected]>



________________________________________
发件人: Hillf Danton <[email protected]>
发送时间: 2021年5月24日 16:25
收件人: Zhang, Qiang
抄送: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
主题: 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 <[email protected]>
>
> 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: [email protected]
> Signed-off-by: Zqiang <[email protected]>
> ---
>  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

  parent reply	other threads:[~2021-05-24  9:19 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-24  7:18 [PATCH] io-wq: Fix UAF when wakeup wqe in hash waitqueue qiang.zhang
     [not found] ` <[email protected]>
2021-05-24  9:19   ` Zhang, Qiang [this message]
2021-05-24 10:16     ` 回复: " Pavel Begunkov
2021-05-25  2:01       ` 回复: " Zhang, Qiang
2021-06-07 17:38         ` Pavel Begunkov
2021-06-10  1:49           ` Zhang, Qiang
2021-05-24 10:18     ` Pavel Begunkov

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=DM6PR11MB4202B442C4C27740B6EE2D64FF269@DM6PR11MB4202.namprd11.prod.outlook.com \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /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