Re: [PATCH 3/9] bfq: Split shared queues on move between cgroups

From: Yu Kuai <yukuai1@huaweicloud.com>
To: Jan Kara <jack@suse.cz>, Yu Kuai <yukuai1@huaweicloud.com>
Cc: linux-block@vger.kernel.org,
	Paolo Valente <paolo.valente@linaro.org>,
	Jens Axboe <axboe@kernel.dk>,
	stable@vger.kernel.org, "yukuai (C)" <yukuai3@huawei.com>
Subject: Re: [PATCH 3/9] bfq: Split shared queues on move between cgroups
Date: Thu, 8 Dec 2022 20:59:24 +0800	[thread overview]
Message-ID: <5a712ac6-e6c7-0008-bee7-2383cc684c73@huaweicloud.com> (raw)
In-Reply-To: <20221208093733.izj7irhzspmvpxxc@quack3>

Hi

在 2022/12/08 17:37, Jan Kara 写道:
> 
> So if this state happens, it would be indeed a problem. But I don't see how
> it could happen. bics are associated with the process. So t1 will always
> use bic1, t2 will always use bic2. In bfq_init_rq() we get bfqq either from
> bic (so it would return bfqq3 for bic1) or we allocate a new queue (that
> would be some bfqq4). So I see no way how bfqq2 could start pointing to
> bic1...

> 
> 								Honza
>

Following is possible scenarios that we derived:

1) Initial state, two process with io.

Process 1       Process 2
  (BIC1)          (BIC2)
   |  Λ           |  Λ
   |  |            |  |
   V  |            V  |
   bfqq1           bfqq2

2) bfqq1 is merged to bfqq2, now bfqq2 has two process ref, bfqq2->bic
    will not be set.

Process 1       Process 2
  (BIC1)          (BIC2)
   |               |
    \-------------\|
                   V
   bfqq1           bfqq2(coop)

3) Process 1 exit, then issue io(denoted IOA) from Process 2.

Process 2
  (BIC1)
   |  Λ
   |  |
   V  |
bfqq2(coop)

4) Before IOA completed, move Process 2 to another cgroup and issue
    io.

Process 2
  (BIC2)
    Λ
    |\--------------\
    |                V
bfqq2(coop)      bfqq3

Now that BIC2 point to bfqq3, while bfqq2 and bfqq3 both point to BIC2.

5) If all the io are completed and Process 2 exit, BIC2 will be freed,
    while bfqq2 still ponits to BIC2.

It's easy to construct such scenario, however, I'm not able to trigger
such UAF yet. It seems hard to let bfqq2 become in_service_queue again
and access bfqq2->bic in bfq_select_queue.

While I'm organizing the above procedures, I realized that my former
solution is wrong. Now I think that the right thing to do is to also
clear bfqq->bic while process ref is decreased to 0.

Thanks,
Kuai