linux-kernel - Re: [PATCH] blk-throttle: check policy bit in blk_throtl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <59464ad0-856c-4ec3-b5b7-e7799c337a84@yukuai.org.cn>
Date: Wed, 3 Sep 2025 01:06:05 +0800
From: Yu Kuai <hailan@...uai.org.cn>
To: gj.han@...mail.com, Jens Axboe <axboe@...nel.dk>,
 "open list:BLOCK LAYER" <linux-block@...r.kernel.org>,
 open list <linux-kernel@...r.kernel.org>
Cc: hanguangjiang@...iang.com, fanggeng@...iang.com, yangchen11@...iang.com,
 liangjie@...iang.com
Subject: Re: [PATCH] blk-throttle: check policy bit in blk_throtl_activated()

Hi,

在 2025/9/2 20:39, gj.han@...mail.com 写道:
> From: Han Guangjiang <hanguangjiang@...iang.com>
>
> On repeated cold boots we occasionally hit a NULL pointer crash in
> blk_should_throtl() when throttling is consulted before the throttle
> policy is fully enabled for the queue. Checking only q->td != NULL is
> insufficient during early initialization, so blkg_to_pd() for the
> throttle policy can still return NULL and blkg_to_tg() becomes NULL,
> which later gets dereferenced.
>
>   Unable to handle kernel NULL pointer dereference
>   at virtual address 0000000000000156
>   ...
>   pc : submit_bio_noacct+0x14c/0x4c8
>   lr : submit_bio_noacct+0x48/0x4c8
>   sp : ffff800087f0b690
>   x29: ffff800087f0b690 x28: 0000000000005f90 x27: ffff00068af393c0
>   x26: 0000000000080000 x25: 000000000002fbc0 x24: ffff000684ddcc70
>   x23: 0000000000000000 x22: 0000000000000000 x21: 0000000000000000
>   x20: 0000000000080000 x19: ffff000684ddcd08 x18: ffffffffffffffff
>   x17: 0000000000000000 x16: ffff80008132a550 x15: 0000ffff98020fff
>   x14: 0000000000000000 x13: 1fffe000d11d7021 x12: ffff000688eb810c
>   x11: ffff00077ec4bb80 x10: ffff000688dcb720 x9 : ffff80008068ef60
>   x8 : 00000a6fb8a86e85 x7 : 000000000000111e x6 : 0000000000000002
>   x5 : 0000000000000246 x4 : 0000000000015cff x3 : 0000000000394500
>   x2 : ffff000682e35e40 x1 : 0000000000364940 x0 : 000000000000001a
>   Call trace:
>    submit_bio_noacct+0x14c/0x4c8
>    verity_map+0x178/0x2c8
>    __map_bio+0x228/0x250
>    dm_submit_bio+0x1c4/0x678
>    __submit_bio+0x170/0x230
>    submit_bio_noacct_nocheck+0x16c/0x388
>    submit_bio_noacct+0x16c/0x4c8
>    submit_bio+0xb4/0x210
>    f2fs_submit_read_bio+0x4c/0xf0
>    f2fs_mpage_readpages+0x3b0/0x5f0
>    f2fs_readahead+0x90/0xe8
>
> Tighten blk_throtl_activated() to also require that the throttle policy
> bit is set on the queue:
>
>    return q->td != NULL &&
>           test_bit(blkcg_policy_throtl.plid, q->blkcg_pols);
>
> This prevents blk_should_throtl() from accessing throttle group state
> until policy data has been attached to blkgs.
>
> Fixes: a3166c51702b ("blk-throttle: delay initialization until configuration")
> Co-developed-by: Liang Jie <liangjie@...iang.com>
> Signed-off-by: Liang Jie <liangjie@...iang.com>
> Signed-off-by: Han Guangjiang <hanguangjiang@...iang.com>
> ---
>   block/blk-throttle.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/blk-throttle.h b/block/blk-throttle.h
> index 3b27755bfbff..9ca43dc56eda 100644
> --- a/block/blk-throttle.h
> +++ b/block/blk-throttle.h
> @@ -156,7 +156,7 @@ void blk_throtl_cancel_bios(struct gendisk *disk);
>   
>   static inline bool blk_throtl_activated(struct request_queue *q)
>   {
> -	return q->td != NULL;
> +	return q->td != NULL && test_bit(blkcg_policy_throtl.plid, q->blkcg_pols);
>   }

Instead of add checking from hot path, do you consider delaying setting q->td
until policy is activated from the slow path? I think this is better solution.

Thanks,
Kuai

>   
>   static inline bool blk_should_throtl(struct bio *bio)