* Re: [BUG] block/bfq-wf2q: A Use After Free bug in bfq_del_bfqq_busy
[not found] <24de85db.8011.1781a216e77.Coremail.lyl2019@mail.ustc.edu.cn>
@ 2021-03-12 14:47 ` Paolo Valente
2021-03-13 14:00 ` lyl2019
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Valente @ 2021-03-12 14:47 UTC (permalink / raw)
To: lyl2019; +Cc: Jens Axboe, linux-block, security
> Il giorno 10 mar 2021, alle ore 04:15, lyl2019@mail.ustc.edu.cn ha scritto:
>
> File: block/bfq-wf2q.c
>
> There exist a feasible path to trigger a use after free bug in
> bfq_del_bfqq_busy, since v4.12-rc1. It could cause denial of service.
>
Thank you very much for analyzing this.
> In the implemention of bfq_del_bfqq_busy,
I've checked all invocations of bfq_del_bfqq_busy, comments below.
> it calls bfq_deactivate_bfqq()
> and use `bfqq` later. Whereas bfq_deactivate_bfqq() could free `bfqq`.
>
> The trigger path is as follow:
> |- bfq_deactivate_bfqq(.., bfqq, true, ..)
> |-- entity = &bfqq->entity; // get entity
> |-- bfq_deactivate_entity(entity, true, ...); //has a path to free `bfqq`
> |-- if (!bfqq->dispatched) // use after free!
>
>
> |- bfq_deactivate_entity(entity, true, ...)
> |-- ...
> |-- for_each_entity_safe(entity, parent) { // in the first loop,
> //entity is the same as before
> if (!__bfq_deactivate_entity(entity, true)) {
>
> |- __bfq_deactivate_entity(entity, true)
> |-- ...
> |-- if (!ins_into_idle_tree || !bfq_gt(entity->finish, st->vtime))
> bfq_forget_entity(st, entity, is_in_service);
>
> |- bfq_forget_entity(st, entity, is_in_service)
> |-- bfqq = bfq_entity_to_bfqq(entity); // recover `bfqq` by entity
> |-- if (bfqq && !is_in_service)
> bfq_put_queue(bfqq); // free the `bfqq`
>
For this put to turn into a free, bfqq should have only one ref. But
I did not find any invocation of bfq_del_bfqq_busy with bfqq->ref == 1.
Did you spot any?
Looking forward to your feedback,
Paolo
> The bug fix needs to add some checks to avoid freeing `bfqq` in the first
> loop in __bfq_deactivate_entity(). I can't come out a good patch for it,
> so i report it for you.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Re: [BUG] block/bfq-wf2q: A Use After Free bug in bfq_del_bfqq_busy
2021-03-12 14:47 ` [BUG] block/bfq-wf2q: A Use After Free bug in bfq_del_bfqq_busy Paolo Valente
@ 2021-03-13 14:00 ` lyl2019
2021-03-16 7:55 ` Paolo Valente
0 siblings, 1 reply; 4+ messages in thread
From: lyl2019 @ 2021-03-13 14:00 UTC (permalink / raw)
To: Paolo Valente; +Cc: Jens Axboe, linux-block, security
> -----原始邮件-----
> 发件人: "Paolo Valente" <paolo.valente@linaro.org>
> 发送时间: 2021-03-12 22:47:22 (星期五)
> 收件人: lyl2019@mail.ustc.edu.cn
> 抄送: "Jens Axboe" <axboe@kernel.dk>, linux-block@vger.kernel.org, security@kernel.org
> 主题: Re: [BUG] block/bfq-wf2q: A Use After Free bug in bfq_del_bfqq_busy
>
>
>
> > Il giorno 10 mar 2021, alle ore 04:15, lyl2019@mail.ustc.edu.cn ha scritto:
> >
> > File: block/bfq-wf2q.c
> >
> > There exist a feasible path to trigger a use after free bug in
> > bfq_del_bfqq_busy, since v4.12-rc1. It could cause denial of service.
> >
>
> Thank you very much for analyzing this.
>
> > In the implemention of bfq_del_bfqq_busy,
>
> I've checked all invocations of bfq_del_bfqq_busy, comments below.
>
> > it calls bfq_deactivate_bfqq()
> > and use `bfqq` later. Whereas bfq_deactivate_bfqq() could free `bfqq`.
> >
> > The trigger path is as follow:
> > |- bfq_deactivate_bfqq(.., bfqq, true, ..)
> > |-- entity = &bfqq->entity; // get entity
> > |-- bfq_deactivate_entity(entity, true, ...); //has a path to free `bfqq`
> > |-- if (!bfqq->dispatched) // use after free!
> >
> >
> > |- bfq_deactivate_entity(entity, true, ...)
> > |-- ...
> > |-- for_each_entity_safe(entity, parent) { // in the first loop,
> > //entity is the same as before
> > if (!__bfq_deactivate_entity(entity, true)) {
> >
> > |- __bfq_deactivate_entity(entity, true)
> > |-- ...
> > |-- if (!ins_into_idle_tree || !bfq_gt(entity->finish, st->vtime))
> > bfq_forget_entity(st, entity, is_in_service);
> >
> > |- bfq_forget_entity(st, entity, is_in_service)
> > |-- bfqq = bfq_entity_to_bfqq(entity); // recover `bfqq` by entity
> > |-- if (bfqq && !is_in_service)
> > bfq_put_queue(bfqq); // free the `bfqq`
> >
>
> For this put to turn into a free, bfqq should have only one ref. But
> I did not find any invocation of bfq_del_bfqq_busy with bfqq->ref == 1.
>
> Did you spot any?
>
> Looking forward to your feedback,
> Paolo
>
> > The bug fix needs to add some checks to avoid freeing `bfqq` in the first
> > loop in __bfq_deactivate_entity(). I can't come out a good patch for it,
> > so i report it for you.
>
I the file block/bfq-iosched.c, function bfq_remove_request get bfqq by
"RQ_BFQQ(rq);". Can we assume bfqq->ref ==1 at this time? After bfq_remove_request
got the bfqq, there is no ref increase operation before calls bfq_del_bfqq_busy().
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [BUG] block/bfq-wf2q: A Use After Free bug in bfq_del_bfqq_busy
2021-03-13 14:00 ` lyl2019
@ 2021-03-16 7:55 ` Paolo Valente
2021-03-17 11:38 ` lyl2019
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Valente @ 2021-03-16 7:55 UTC (permalink / raw)
To: lyl2019; +Cc: Jens Axboe, linux-block, security
> Il giorno 13 mar 2021, alle ore 15:00, lyl2019@mail.ustc.edu.cn ha scritto:
>
>
>
>
>> -----原始邮件-----
>> 发件人: "Paolo Valente" <paolo.valente@linaro.org>
>> 发送时间: 2021-03-12 22:47:22 (星期五)
>> 收件人: lyl2019@mail.ustc.edu.cn
>> 抄送: "Jens Axboe" <axboe@kernel.dk>, linux-block@vger.kernel.org, security@kernel.org
>> 主题: Re: [BUG] block/bfq-wf2q: A Use After Free bug in bfq_del_bfqq_busy
>>
>>
>>
>>> Il giorno 10 mar 2021, alle ore 04:15, lyl2019@mail.ustc.edu.cn ha scritto:
>>>
>>> File: block/bfq-wf2q.c
>>>
>>> There exist a feasible path to trigger a use after free bug in
>>> bfq_del_bfqq_busy, since v4.12-rc1. It could cause denial of service.
>>>
>>
>> Thank you very much for analyzing this.
>>
>>> In the implemention of bfq_del_bfqq_busy,
>>
>> I've checked all invocations of bfq_del_bfqq_busy, comments below.
>>
>>> it calls bfq_deactivate_bfqq()
>>> and use `bfqq` later. Whereas bfq_deactivate_bfqq() could free `bfqq`.
>>>
>>> The trigger path is as follow:
>>> |- bfq_deactivate_bfqq(.., bfqq, true, ..)
>>> |-- entity = &bfqq->entity; // get entity
>>> |-- bfq_deactivate_entity(entity, true, ...); //has a path to free `bfqq`
>>> |-- if (!bfqq->dispatched) // use after free!
>>>
>>>
>>> |- bfq_deactivate_entity(entity, true, ...)
>>> |-- ...
>>> |-- for_each_entity_safe(entity, parent) { // in the first loop,
>>> //entity is the same as before
>>> if (!__bfq_deactivate_entity(entity, true)) {
>>>
>>> |- __bfq_deactivate_entity(entity, true)
>>> |-- ...
>>> |-- if (!ins_into_idle_tree || !bfq_gt(entity->finish, st->vtime))
>>> bfq_forget_entity(st, entity, is_in_service);
>>>
>>> |- bfq_forget_entity(st, entity, is_in_service)
>>> |-- bfqq = bfq_entity_to_bfqq(entity); // recover `bfqq` by entity
>>> |-- if (bfqq && !is_in_service)
>>> bfq_put_queue(bfqq); // free the `bfqq`
>>>
>>
>> For this put to turn into a free, bfqq should have only one ref. But
>> I did not find any invocation of bfq_del_bfqq_busy with bfqq->ref == 1.
>>
>> Did you spot any?
>>
>> Looking forward to your feedback,
>> Paolo
>>
>>> The bug fix needs to add some checks to avoid freeing `bfqq` in the first
>>> loop in __bfq_deactivate_entity(). I can't come out a good patch for it,
>>> so i report it for you.
>>
>
> I the file block/bfq-iosched.c, function bfq_remove_request get bfqq by
> "RQ_BFQQ(rq);". Can we assume bfqq->ref ==1 at this time? After bfq_remove_request
> got the bfqq, there is no ref increase operation before calls bfq_del_bfqq_busy().
>
The scheme is: a ref is taken for a request arrival into a bfq_queue,
in __bfq_insert_request, and then that ref is released on the
completion of that request, in bfq_finish_requeue_request_body. In
this respect, bfq_remove_request is invoked for a request only before
bfq_finish_requeue_request_body is invoked for the same request. So
the bfq_del_bfqq_busy invocation that you highlight should not lead to
a free, because the ref counter should remain always higher than 0.
If you think I'm missing something (which is possible and welcome!),
try to write the sequence of events that leads to the free you
suspect, and to show that the counter actually reaches zero. Recall
that a deactivate should be paired with an activation, and an
activation should always cause the ref counter to be increased.
Looking forward to your feedback,
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Re: [BUG] block/bfq-wf2q: A Use After Free bug in bfq_del_bfqq_busy
2021-03-16 7:55 ` Paolo Valente
@ 2021-03-17 11:38 ` lyl2019
0 siblings, 0 replies; 4+ messages in thread
From: lyl2019 @ 2021-03-17 11:38 UTC (permalink / raw)
To: Paolo Valente; +Cc: Jens Axboe, linux-block, security
Ok, i think it is a false positive now.
Thanks for your time and patient answer.
> -----原始邮件-----
> 发件人: "Paolo Valente" <paolo.valente@linaro.org>
> 发送时间: 2021-03-16 15:55:44 (星期二)
> 收件人: lyl2019@mail.ustc.edu.cn
> 抄送: "Jens Axboe" <axboe@kernel.dk>, linux-block@vger.kernel.org, security@kernel.org
> 主题: Re: [BUG] block/bfq-wf2q: A Use After Free bug in bfq_del_bfqq_busy
>
>
>
> > Il giorno 13 mar 2021, alle ore 15:00, lyl2019@mail.ustc.edu.cn ha scritto:
> >
> >
> >
> >
> >> -----原始邮件-----
> >> 发件人: "Paolo Valente" <paolo.valente@linaro.org>
> >> 发送时间: 2021-03-12 22:47:22 (星期五)
> >> 收件人: lyl2019@mail.ustc.edu.cn
> >> 抄送: "Jens Axboe" <axboe@kernel.dk>, linux-block@vger.kernel.org, security@kernel.org
> >> 主题: Re: [BUG] block/bfq-wf2q: A Use After Free bug in bfq_del_bfqq_busy
> >>
> >>
> >>
> >>> Il giorno 10 mar 2021, alle ore 04:15, lyl2019@mail.ustc.edu.cn ha scritto:
> >>>
> >>> File: block/bfq-wf2q.c
> >>>
> >>> There exist a feasible path to trigger a use after free bug in
> >>> bfq_del_bfqq_busy, since v4.12-rc1. It could cause denial of service.
> >>>
> >>
> >> Thank you very much for analyzing this.
> >>
> >>> In the implemention of bfq_del_bfqq_busy,
> >>
> >> I've checked all invocations of bfq_del_bfqq_busy, comments below.
> >>
> >>> it calls bfq_deactivate_bfqq()
> >>> and use `bfqq` later. Whereas bfq_deactivate_bfqq() could free `bfqq`.
> >>>
> >>> The trigger path is as follow:
> >>> |- bfq_deactivate_bfqq(.., bfqq, true, ..)
> >>> |-- entity = &bfqq->entity; // get entity
> >>> |-- bfq_deactivate_entity(entity, true, ...); //has a path to free `bfqq`
> >>> |-- if (!bfqq->dispatched) // use after free!
> >>>
> >>>
> >>> |- bfq_deactivate_entity(entity, true, ...)
> >>> |-- ...
> >>> |-- for_each_entity_safe(entity, parent) { // in the first loop,
> >>> //entity is the same as before
> >>> if (!__bfq_deactivate_entity(entity, true)) {
> >>>
> >>> |- __bfq_deactivate_entity(entity, true)
> >>> |-- ...
> >>> |-- if (!ins_into_idle_tree || !bfq_gt(entity->finish, st->vtime))
> >>> bfq_forget_entity(st, entity, is_in_service);
> >>>
> >>> |- bfq_forget_entity(st, entity, is_in_service)
> >>> |-- bfqq = bfq_entity_to_bfqq(entity); // recover `bfqq` by entity
> >>> |-- if (bfqq && !is_in_service)
> >>> bfq_put_queue(bfqq); // free the `bfqq`
> >>>
> >>
> >> For this put to turn into a free, bfqq should have only one ref. But
> >> I did not find any invocation of bfq_del_bfqq_busy with bfqq->ref == 1.
> >>
> >> Did you spot any?
> >>
> >> Looking forward to your feedback,
> >> Paolo
> >>
> >>> The bug fix needs to add some checks to avoid freeing `bfqq` in the first
> >>> loop in __bfq_deactivate_entity(). I can't come out a good patch for it,
> >>> so i report it for you.
> >>
> >
> > I the file block/bfq-iosched.c, function bfq_remove_request get bfqq by
> > "RQ_BFQQ(rq);". Can we assume bfqq->ref ==1 at this time? After bfq_remove_request
> > got the bfqq, there is no ref increase operation before calls bfq_del_bfqq_busy().
> >
>
> The scheme is: a ref is taken for a request arrival into a bfq_queue,
> in __bfq_insert_request, and then that ref is released on the
> completion of that request, in bfq_finish_requeue_request_body. In
> this respect, bfq_remove_request is invoked for a request only before
> bfq_finish_requeue_request_body is invoked for the same request. So
> the bfq_del_bfqq_busy invocation that you highlight should not lead to
> a free, because the ref counter should remain always higher than 0.
> If you think I'm missing something (which is possible and welcome!),
> try to write the sequence of events that leads to the free you
> suspect, and to show that the counter actually reaches zero. Recall
> that a deactivate should be paired with an activation, and an
> activation should always cause the ref counter to be increased.
>
> Looking forward to your feedback,
> Paolo
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-17 11:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <24de85db.8011.1781a216e77.Coremail.lyl2019@mail.ustc.edu.cn>
2021-03-12 14:47 ` [BUG] block/bfq-wf2q: A Use After Free bug in bfq_del_bfqq_busy Paolo Valente
2021-03-13 14:00 ` lyl2019
2021-03-16 7:55 ` Paolo Valente
2021-03-17 11:38 ` lyl2019
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).