* Re: [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq()
2023-01-13 9:44 [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq() Yu Kuai
@ 2023-01-13 9:46 ` Jan Kara
2023-01-13 13:38 ` Holger Hoffstätte
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Jan Kara @ 2023-01-13 9:46 UTC (permalink / raw)
To: Yu Kuai
Cc: jack, tj, josef, axboe, paolo.valente, shinichiro.kawasaki,
cgroups, linux-block, linux-kernel, yukuai1, yi.zhang, yangerkun
On Fri 13-01-23 17:44:10, Yu Kuai wrote:
> After commit 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'"),
> bic->bfqq will be accessed in bic_set_bfqq(), however, in some context
> bic->bfqq will be freed first, and bic_set_bfqq() is called with the freed
> bic->bfqq.
>
> Fix the problem by always freeing bfqq after bic_set_bfqq().
>
> Fixes: 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'")
> Reported-and-tested-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
Looks good, thanks for the fix! Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> block/bfq-cgroup.c | 2 +-
> block/bfq-iosched.c | 4 +++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
> index a6e8da5f5cfd..feb13ac25557 100644
> --- a/block/bfq-cgroup.c
> +++ b/block/bfq-cgroup.c
> @@ -749,8 +749,8 @@ static void bfq_sync_bfqq_move(struct bfq_data *bfqd,
> * old cgroup.
> */
> bfq_put_cooperator(sync_bfqq);
> - bfq_release_process_ref(bfqd, sync_bfqq);
> bic_set_bfqq(bic, NULL, true, act_idx);
> + bfq_release_process_ref(bfqd, sync_bfqq);
> }
> }
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 815b884d6c5a..2ddf831221c4 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -5581,9 +5581,11 @@ static void bfq_check_ioprio_change(struct bfq_io_cq *bic, struct bio *bio)
>
> bfqq = bic_to_bfqq(bic, false, bfq_actuator_index(bfqd, bio));
> if (bfqq) {
> - bfq_release_process_ref(bfqd, bfqq);
> + struct bfq_queue *old_bfqq = bfqq;
> +
> bfqq = bfq_get_queue(bfqd, bio, false, bic, true);
> bic_set_bfqq(bic, bfqq, false, bfq_actuator_index(bfqd, bio));
> + bfq_release_process_ref(bfqd, old_bfqq);
> }
>
> bfqq = bic_to_bfqq(bic, true, bfq_actuator_index(bfqd, bio));
> --
> 2.31.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq()
2023-01-13 9:44 [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq() Yu Kuai
2023-01-13 9:46 ` Jan Kara
@ 2023-01-13 13:38 ` Holger Hoffstätte
2023-01-16 3:09 ` Yu Kuai
2023-01-24 0:09 ` Shinichiro Kawasaki
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Holger Hoffstätte @ 2023-01-13 13:38 UTC (permalink / raw)
To: Yu Kuai, Jan Kara, linux-block, Jens Axboe, Paolo Valente
On 2023-01-13 10:44, Yu Kuai wrote:
> After commit 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'"),
> bic->bfqq will be accessed in bic_set_bfqq(), however, in some context
> bic->bfqq will be freed first, and bic_set_bfqq() is called with the freed
> bic->bfqq.
>
> Fix the problem by always freeing bfqq after bic_set_bfqq().
>
> Fixes: 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'")
> Reported-and-tested-by: Shinichiro Kawasaki <shinichiro.kawasaki-Sjgp3cTcYWE@public.gmane.org>
> Signed-off-by: Yu Kuai <yukuai3-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
> block/bfq-cgroup.c | 2 +-
> block/bfq-iosched.c | 4 +++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
> index a6e8da5f5cfd..feb13ac25557 100644
> --- a/block/bfq-cgroup.c
> +++ b/block/bfq-cgroup.c
> @@ -749,8 +749,8 @@ static void bfq_sync_bfqq_move(struct bfq_data *bfqd,
> * old cgroup.
> */
> bfq_put_cooperator(sync_bfqq);
> - bfq_release_process_ref(bfqd, sync_bfqq);
> bic_set_bfqq(bic, NULL, true, act_idx);
> + bfq_release_process_ref(bfqd, sync_bfqq);
> }
> }
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 815b884d6c5a..2ddf831221c4 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -5581,9 +5581,11 @@ static void bfq_check_ioprio_change(struct bfq_io_cq *bic, struct bio *bio)
>
> bfqq = bic_to_bfqq(bic, false, bfq_actuator_index(bfqd, bio));
> if (bfqq) {
> - bfq_release_process_ref(bfqd, bfqq);
> + struct bfq_queue *old_bfqq = bfqq;
> +
> bfqq = bfq_get_queue(bfqd, bio, false, bic, true);
> bic_set_bfqq(bic, bfqq, false, bfq_actuator_index(bfqd, bio));
> + bfq_release_process_ref(bfqd, old_bfqq);
> }
>
> bfqq = bic_to_bfqq(bic, true, bfq_actuator_index(bfqd, bio));
>
Hello,
does this condition also affect the current code? I ask since the patch does not apply
as bfq_sync_bfqq_move() is only part of the multi-actuator work, which is only in
Jens' for-next. Comparing the code sections it seems it should also be necessary for
current 6.1/6.2, but I wanted to check.
thanks
Holger
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq()
2023-01-13 13:38 ` Holger Hoffstätte
@ 2023-01-16 3:09 ` Yu Kuai
0 siblings, 0 replies; 12+ messages in thread
From: Yu Kuai @ 2023-01-16 3:09 UTC (permalink / raw)
To: Holger Hoffstätte, Jan Kara, linux-block, Jens Axboe,
Paolo Valente, yukuai (C)
Hi,
在 2023/01/13 21:38, Holger Hoffstätte 写道:
> On 2023-01-13 10:44, Yu Kuai wrote:
>> After commit 64dc8c732f5c ("block, bfq: fix possible uaf for
>> 'bfqq->bic'"),
>> bic->bfqq will be accessed in bic_set_bfqq(), however, in some context
>> bic->bfqq will be freed first, and bic_set_bfqq() is called with the
>> freed
>> bic->bfqq.
>>
>> Fix the problem by always freeing bfqq after bic_set_bfqq().
>>
>> Fixes: 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'")
>> Reported-and-tested-by: Shinichiro Kawasaki
>> <shinichiro.kawasaki-Sjgp3cTcYWE@public.gmane.org>
>> Signed-off-by: Yu Kuai <yukuai3-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>> block/bfq-cgroup.c | 2 +-
>> block/bfq-iosched.c | 4 +++-
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
>> index a6e8da5f5cfd..feb13ac25557 100644
>> --- a/block/bfq-cgroup.c
>> +++ b/block/bfq-cgroup.c
>> @@ -749,8 +749,8 @@ static void bfq_sync_bfqq_move(struct bfq_data *bfqd,
>> * old cgroup.
>> */
>> bfq_put_cooperator(sync_bfqq);
>> - bfq_release_process_ref(bfqd, sync_bfqq);
>> bic_set_bfqq(bic, NULL, true, act_idx);
>> + bfq_release_process_ref(bfqd, sync_bfqq);
>> }
>> }
>> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
>> index 815b884d6c5a..2ddf831221c4 100644
>> --- a/block/bfq-iosched.c
>> +++ b/block/bfq-iosched.c
>> @@ -5581,9 +5581,11 @@ static void bfq_check_ioprio_change(struct
>> bfq_io_cq *bic, struct bio *bio)
>> bfqq = bic_to_bfqq(bic, false, bfq_actuator_index(bfqd, bio));
>> if (bfqq) {
>> - bfq_release_process_ref(bfqd, bfqq);
>> + struct bfq_queue *old_bfqq = bfqq;
>> +
>> bfqq = bfq_get_queue(bfqd, bio, false, bic, true);
>> bic_set_bfqq(bic, bfqq, false, bfq_actuator_index(bfqd, bio));
>> + bfq_release_process_ref(bfqd, old_bfqq);
>> }
>> bfqq = bic_to_bfqq(bic, true, bfq_actuator_index(bfqd, bio));
>>
>
> Hello,
>
> does this condition also affect the current code? I ask since the patch
> does not apply
> as bfq_sync_bfqq_move() is only part of the multi-actuator work, which
> is only in
> Jens' for-next. Comparing the code sections it seems it should also be
> necessary for
> current 6.1/6.2, but I wanted to check.
bfq_sync_bfqq_move() already make sure bfq_release_process_ref() is
called after bic_set_bfqq(), so the problem this patch tries to fix
should not exist here.
Thanks,
Kuai
>
> thanks
> Holger
>
> .
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq()
2023-01-13 9:44 [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq() Yu Kuai
2023-01-13 9:46 ` Jan Kara
2023-01-13 13:38 ` Holger Hoffstätte
@ 2023-01-24 0:09 ` Shinichiro Kawasaki
2023-01-29 1:18 ` Yu Kuai
2023-01-29 1:38 ` Yu Kuai
2023-02-21 7:04 ` Yu Kuai
4 siblings, 1 reply; 12+ messages in thread
From: Shinichiro Kawasaki @ 2023-01-24 0:09 UTC (permalink / raw)
To: Yu Kuai
Cc: jack@suse.cz, tj@kernel.org, josef@toxicpanda.com,
axboe@kernel.dk, paolo.valente@linaro.org,
cgroups@vger.kernel.org, linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org, yukuai1@huaweicloud.com,
yi.zhang@huawei.com, yangerkun@huawei.com
On Jan 13, 2023 / 17:44, Yu Kuai wrote:
> After commit 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'"),
> bic->bfqq will be accessed in bic_set_bfqq(), however, in some context
> bic->bfqq will be freed first, and bic_set_bfqq() is called with the freed
> bic->bfqq.
>
> Fix the problem by always freeing bfqq after bic_set_bfqq().
>
> Fixes: 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'")
> Reported-and-tested-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
> block/bfq-cgroup.c | 2 +-
> block/bfq-iosched.c | 4 +++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
> index a6e8da5f5cfd..feb13ac25557 100644
> --- a/block/bfq-cgroup.c
> +++ b/block/bfq-cgroup.c
> @@ -749,8 +749,8 @@ static void bfq_sync_bfqq_move(struct bfq_data *bfqd,
> * old cgroup.
> */
> bfq_put_cooperator(sync_bfqq);
> - bfq_release_process_ref(bfqd, sync_bfqq);
> bic_set_bfqq(bic, NULL, true, act_idx);
> + bfq_release_process_ref(bfqd, sync_bfqq);
> }
> }
>
Yu, thanks for posting this fix, but it can not be applied to v6.2-rc5. The
hunk above looks different from the patch I tested. Could you take a look?
--
Shin'ichiro Kawasaki
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq()
2023-01-24 0:09 ` Shinichiro Kawasaki
@ 2023-01-29 1:18 ` Yu Kuai
0 siblings, 0 replies; 12+ messages in thread
From: Yu Kuai @ 2023-01-29 1:18 UTC (permalink / raw)
To: Shinichiro Kawasaki
Cc: jack@suse.cz, tj@kernel.org, josef@toxicpanda.com,
axboe@kernel.dk, paolo.valente@linaro.org,
cgroups@vger.kernel.org, linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org, yukuai1@huaweicloud.com,
yi.zhang@huawei.com, yangerkun@huawei.com, yukuai (C)
Hi,
在 2023/01/24 8:09, Shinichiro Kawasaki 写道:
>
> Yu, thanks for posting this fix, but it can not be applied to v6.2-rc5. The
> hunk above looks different from the patch I tested. Could you take a look?
>
This patch was rebased with following patch that add a new param for
bic_set_bfqq():
51ec2387623a block, bfq: split sync bfq_queues on a per-actuator basis
Thanks,
Kuai
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq()
2023-01-13 9:44 [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq() Yu Kuai
` (2 preceding siblings ...)
2023-01-24 0:09 ` Shinichiro Kawasaki
@ 2023-01-29 1:38 ` Yu Kuai
2023-01-29 21:51 ` Jens Axboe
2023-02-21 7:04 ` Yu Kuai
4 siblings, 1 reply; 12+ messages in thread
From: Yu Kuai @ 2023-01-29 1:38 UTC (permalink / raw)
To: jack, tj, josef, axboe, paolo.valente, shinichiro.kawasaki
Cc: cgroups, linux-block, linux-kernel, yukuai1, yi.zhang, yangerkun,
yukuai (C)
Hi, Jens
在 2023/01/13 17:44, Yu Kuai 写道:
> After commit 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'"),
> bic->bfqq will be accessed in bic_set_bfqq(), however, in some context
> bic->bfqq will be freed first, and bic_set_bfqq() is called with the freed
> bic->bfqq.
>
> Fix the problem by always freeing bfqq after bic_set_bfqq().
>
Sorry that I send this patch will wrong email, and you might missed this
patch.
Can you apply this patch? This patch can't be applied directly to lower
version due to Paolo's patchset, I'll send lts patch seperately.
Thanks,
Kuai
> Fixes: 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'")
> Reported-and-tested-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq()
2023-01-29 1:38 ` Yu Kuai
@ 2023-01-29 21:51 ` Jens Axboe
2023-01-30 1:06 ` Yu Kuai
0 siblings, 1 reply; 12+ messages in thread
From: Jens Axboe @ 2023-01-29 21:51 UTC (permalink / raw)
To: Yu Kuai, jack, tj, josef, paolo.valente, shinichiro.kawasaki
Cc: cgroups, linux-block, linux-kernel, yi.zhang, yangerkun,
yukuai (C)
On 1/28/23 6:38 PM, Yu Kuai wrote:
> Hi, Jens
>
> 在 2023/01/13 17:44, Yu Kuai 写道:
>> After commit 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'"),
>> bic->bfqq will be accessed in bic_set_bfqq(), however, in some context
>> bic->bfqq will be freed first, and bic_set_bfqq() is called with the freed
>> bic->bfqq.
>>
>> Fix the problem by always freeing bfqq after bic_set_bfqq().
>>
>
> Sorry that I send this patch will wrong email, and you might missed this
> patch.
>
> Can you apply this patch? This patch can't be applied directly to lower
> version due to Paolo's patchset, I'll send lts patch seperately.
I'm confused... So this patch only applies to the 6.3 branch, yet we
need it in 6.2 as far as I can tell. Why isn't it against block-6.2
then?
--
Jens Axboe
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq()
2023-01-29 21:51 ` Jens Axboe
@ 2023-01-30 1:06 ` Yu Kuai
0 siblings, 0 replies; 12+ messages in thread
From: Yu Kuai @ 2023-01-30 1:06 UTC (permalink / raw)
To: Jens Axboe, Yu Kuai, jack, tj, josef, paolo.valente,
shinichiro.kawasaki
Cc: cgroups, linux-block, linux-kernel, yi.zhang, yangerkun,
yukuai (C)
Hi,
在 2023/01/30 5:51, Jens Axboe 写道:
> On 1/28/23 6:38 PM, Yu Kuai wrote:
>> Hi, Jens
>>
>> 在 2023/01/13 17:44, Yu Kuai 写道:
>>> After commit 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'"),
>>> bic->bfqq will be accessed in bic_set_bfqq(), however, in some context
>>> bic->bfqq will be freed first, and bic_set_bfqq() is called with the freed
>>> bic->bfqq.
>>>
>>> Fix the problem by always freeing bfqq after bic_set_bfqq().
>>>
>>
>> Sorry that I send this patch will wrong email, and you might missed this
>> patch.
>>
>> Can you apply this patch? This patch can't be applied directly to lower
>> version due to Paolo's patchset, I'll send lts patch seperately.
>
> I'm confused... So this patch only applies to the 6.3 branch, yet we
> need it in 6.2 as far as I can tell. Why isn't it against block-6.2
> then?
>
Ok, I'll send a new patch against block-6.2.
Thanks,
Kuai
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq()
2023-01-13 9:44 [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq() Yu Kuai
` (3 preceding siblings ...)
2023-01-29 1:38 ` Yu Kuai
@ 2023-02-21 7:04 ` Yu Kuai
2023-02-21 10:19 ` Holger Hoffstätte
4 siblings, 1 reply; 12+ messages in thread
From: Yu Kuai @ 2023-02-21 7:04 UTC (permalink / raw)
To: jack, tj, josef, axboe, paolo.valente, shinichiro.kawasaki
Cc: cgroups, linux-block, linux-kernel, yukuai1, yi.zhang, yangerkun
Hi, Jens
在 2023/01/13 17:44, Yu Kuai 写道:
> After commit 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'"),
> bic->bfqq will be accessed in bic_set_bfqq(), however, in some context
> bic->bfqq will be freed first, and bic_set_bfqq() is called with the freed
> bic->bfqq.
>
> Fix the problem by always freeing bfqq after bic_set_bfqq().
>
> Fixes: 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'")
> Reported-and-tested-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
> block/bfq-cgroup.c | 2 +-
> block/bfq-iosched.c | 4 +++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
> index a6e8da5f5cfd..feb13ac25557 100644
> --- a/block/bfq-cgroup.c
> +++ b/block/bfq-cgroup.c
> @@ -749,8 +749,8 @@ static void bfq_sync_bfqq_move(struct bfq_data *bfqd,
> * old cgroup.
> */
> bfq_put_cooperator(sync_bfqq);
> - bfq_release_process_ref(bfqd, sync_bfqq);
> bic_set_bfqq(bic, NULL, true, act_idx);
> + bfq_release_process_ref(bfqd, sync_bfqq);
> }
> }
>
It seems this change is missed in GIT PULL for-6.3. I'll send a seperate
patch to fix this...
Thanks,
Kuai
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq()
2023-02-21 7:04 ` Yu Kuai
@ 2023-02-21 10:19 ` Holger Hoffstätte
2023-02-21 13:36 ` Yu Kuai
0 siblings, 1 reply; 12+ messages in thread
From: Holger Hoffstätte @ 2023-02-21 10:19 UTC (permalink / raw)
To: Yu Kuai, jack, tj, josef, axboe, paolo.valente,
shinichiro.kawasaki
Cc: cgroups, linux-block, linux-kernel, yi.zhang, yangerkun
On 2023-02-21 08:04, Yu Kuai wrote:
> Hi, Jens
>
> 在 2023/01/13 17:44, Yu Kuai 写道:
>> After commit 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'"),
>> bic->bfqq will be accessed in bic_set_bfqq(), however, in some context
>> bic->bfqq will be freed first, and bic_set_bfqq() is called with the freed
>> bic->bfqq.
>>
>> Fix the problem by always freeing bfqq after bic_set_bfqq().
>>
>> Fixes: 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'")
>> Reported-and-tested-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>> ---
>> block/bfq-cgroup.c | 2 +-
>> block/bfq-iosched.c | 4 +++-
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
>> index a6e8da5f5cfd..feb13ac25557 100644
>> --- a/block/bfq-cgroup.c
>> +++ b/block/bfq-cgroup.c
>> @@ -749,8 +749,8 @@ static void bfq_sync_bfqq_move(struct bfq_data *bfqd,
>> * old cgroup.
>> */
>> bfq_put_cooperator(sync_bfqq);
>> - bfq_release_process_ref(bfqd, sync_bfqq);
>> bic_set_bfqq(bic, NULL, true, act_idx);
>> + bfq_release_process_ref(bfqd, sync_bfqq);
>> }
>> }
>>
>
> It seems this change is missed in GIT PULL for-6.3. I'll send a seperate
> patch to fix this...
>
It was already applied in time for 6.2 as b600de2d7d3a16f9007fad1bdae82a3951a26af2
and also already merged to 6.1-stable.
cheers
Holger
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] block, bfq: fix uaf for bfqq in bic_set_bfqq()
2023-02-21 10:19 ` Holger Hoffstätte
@ 2023-02-21 13:36 ` Yu Kuai
0 siblings, 0 replies; 12+ messages in thread
From: Yu Kuai @ 2023-02-21 13:36 UTC (permalink / raw)
To: Holger Hoffstätte, Yu Kuai, jack, tj, josef, axboe,
paolo.valente, shinichiro.kawasaki
Cc: cgroups, linux-block, linux-kernel, yi.zhang, yangerkun,
yukuai (C)
Hi,
在 2023/02/21 18:19, Holger Hoffstätte 写道:
> On 2023-02-21 08:04, Yu Kuai wrote:
>> Hi, Jens
>>
>> 在 2023/01/13 17:44, Yu Kuai 写道:
>>> After commit 64dc8c732f5c ("block, bfq: fix possible uaf for
>>> 'bfqq->bic'"),
>>> bic->bfqq will be accessed in bic_set_bfqq(), however, in some context
>>> bic->bfqq will be freed first, and bic_set_bfqq() is called with the
>>> freed
>>> bic->bfqq.
>>>
>>> Fix the problem by always freeing bfqq after bic_set_bfqq().
>>>
>>> Fixes: 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'")
>>> Reported-and-tested-by: Shinichiro Kawasaki
>>> <shinichiro.kawasaki@wdc.com>
>>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>>> ---
>>> block/bfq-cgroup.c | 2 +-
>>> block/bfq-iosched.c | 4 +++-
>>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
>>> index a6e8da5f5cfd..feb13ac25557 100644
>>> --- a/block/bfq-cgroup.c
>>> +++ b/block/bfq-cgroup.c
>>> @@ -749,8 +749,8 @@ static void bfq_sync_bfqq_move(struct bfq_data
>>> *bfqd,
>>> * old cgroup.
>>> */
>>> bfq_put_cooperator(sync_bfqq);
>>> - bfq_release_process_ref(bfqd, sync_bfqq);
>>> bic_set_bfqq(bic, NULL, true, act_idx);
>>> + bfq_release_process_ref(bfqd, sync_bfqq);
>>> }
>>> }
>>>
>>
>> It seems this change is missed in GIT PULL for-6.3. I'll send a seperate
>> patch to fix this...
>>
>
> It was already applied in time for 6.2 as
> b600de2d7d3a16f9007fad1bdae82a3951a26af2
> and also already merged to 6.1-stable.
Yes, 6.2 and 6.1 doesn't have such problem because bfq_sync_bfqq_move()
doesn't exist. The problem only exist in master branch currently.
Thanks,
Kuai
>
> cheers
> Holger
>
> .
>
^ permalink raw reply [flat|nested] 12+ messages in thread