From 4667e926ac4a0ca09845ebd78fd96fbdca74d0be Mon Sep 17 00:00:00 2001 From: Joseph Qi Date: Thu, 7 Dec 2017 10:18:21 +0800 Subject: [PATCH] alios: blk-throttle: fix tg NULL pointer dereference io throtl stats will blkg_get at the beginning of throttle and then blkg_put at the new introduced bi_tg_end_io. This will cause blkg to be freed if end_io is called twice like dm-thin, which will save origin end_io first, and call its overwrite end_io and then the saved end_io. After that, access blkg is invalid and finally BUG: [ 4417.235048] BUG: unable to handle kernel NULL pointer dereference at 00000000000001e0 [ 4417.236475] IP: [] throtl_update_dispatch_stats+0x21/0xb0 [ 4417.237865] PGD 98395067 PUD 362e1067 PMD 0 [ 4417.239232] Oops: 0000 [#1] SMP ...... [ 4417.274070] Call Trace: [ 4417.275407] [] blk_throtl_bio+0xfd/0x630 [ 4417.276760] [] ? wake_up_process+0x23/0x40 [ 4417.278079] [] ? wake_up_worker+0x24/0x30 [ 4417.279387] [] ? insert_work+0x62/0xa0 [ 4417.280697] [] ? mempool_free_slab+0x17/0x20 [ 4417.282019] [] ? mempool_free+0x49/0x90 [ 4417.283326] [] generic_make_request_checks+0x16f/0x360 [ 4417.284637] [] ? thin_map+0x227/0x2c0 [dm_thin_pool] [ 4417.285951] [] generic_make_request+0x27/0x130 [ 4417.287240] [] __map_bio+0xad/0x100 [dm_mod] [ 4417.288503] [] __clone_and_map_data_bio+0x15e/0x240 [dm_mod] [ 4417.289778] [] __split_and_process_bio+0x38a/0x500 [dm_mod] [ 4417.291062] [] dm_make_request+0x131/0x1a0 [dm_mod] [ 4417.292344] [] generic_make_request+0xe2/0x130 [ 4417.293626] [] submit_bio+0x71/0x150 [ 4417.294909] [] ? bio_alloc_bioset+0x20d/0x360 [ 4417.296195] [] _submit_bh+0x14b/0x220 [ 4417.297484] [] submit_bh+0x10/0x20 [ 4417.298744] [] jbd2_journal_commit_transaction+0x6c8/0x19a0 [jbd2] [ 4417.300014] [] ? __switch_to+0xf8/0x4c0 [ 4417.301268] [] kjournald2+0xc9/0x270 [jbd2] [ 4417.302524] [] ? wake_up_atomic_t+0x30/0x30 [ 4417.303753] [] ? commit_timeout+0x10/0x10 [jbd2] [ 4417.304950] [] kthread+0xcf/0xe0 [ 4417.306107] [] ? kthread_create_on_node+0x140/0x140 [ 4417.307255] [] ret_from_fork+0x58/0x90 [ 4417.308349] [] ? kthread_create_on_node+0x140/0x140 ...... Now we introduce a new bio flag BIO_THROTL_STATED to make sure blkg_get/put only get called once for the same bio. Signed-off-by: Joseph Qi Reviewed-by: Jiufei Xue Reviewed-by: Xiaoguang Wang Acked-by: Caspar Zhang --- block/blk-throttle.c | 17 +++++++++++++++-- include/linux/blk_types.h | 1 + 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 3836af6d45a2..d3527af8803a 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -1088,6 +1088,10 @@ static void throtl_bio_end_io(struct bio *bio) struct throtl_grp *tg; rcu_read_lock(); + /* see comments in throtl_bio_stats_start() */ + if (bio_flagged(bio, BIO_THROTL_STATED)) + goto out; + tg = (struct throtl_grp *)bio->bi_tg_private; if (!tg) goto out; @@ -1096,6 +1100,7 @@ static void throtl_bio_end_io(struct bio *bio) bio_io_start_time_ns(bio), bio_op(bio)); blkg_put(tg_to_blkg(tg)); + bio_clear_flag(bio, BIO_THROTL_STATED); out: rcu_read_unlock(); } @@ -1104,11 +1109,19 @@ static inline void throtl_bio_stats_start(struct bio *bio, struct throtl_grp *tg { int op = bio_op(bio); - if (op == REQ_OP_READ || op == REQ_OP_WRITE) { + /* + * It may happen that end_io will be called twice like dm-thin, + * which will save origin end_io first, and call its overwrite + * end_io and then the saved end_io. We use bio flag + * BIO_THROTL_STATED to do only once statistics. + */ + if ((op == REQ_OP_READ || op == REQ_OP_WRITE) && + !bio_flagged(bio, BIO_THROTL_STATED)) { + blkg_get(tg_to_blkg(tg)); + bio_set_flag(bio, BIO_THROTL_STATED); bio->bi_tg_end_io = throtl_bio_end_io; bio->bi_tg_private = tg; bio_set_start_time_ns(bio); - blkg_get(tg_to_blkg(tg)); } } diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index dedee389966c..4badbaae2a9c 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -238,6 +238,7 @@ struct bio { #define BIO_TRACE_COMPLETION 10 /* bio_endio() should trace the final completion * of this bio. */ #define BIO_QUEUE_ENTERED 11 /* can use blk_queue_enter_live() */ +#define BIO_THROTL_STATED 12 /* bio already stated */ /* See BVEC_POOL_OFFSET below before adding new flags */ -- GitLab