提交 4667e926 编写于 作者: J Joseph Qi

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: [<ffffffff812e7c71>] 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]  [<ffffffff812ea93d>] blk_throtl_bio+0xfd/0x630
[ 4417.276760]  [<ffffffff810b3613>] ? wake_up_process+0x23/0x40
[ 4417.278079]  [<ffffffff81094c04>] ? wake_up_worker+0x24/0x30
[ 4417.279387]  [<ffffffff81095772>] ? insert_work+0x62/0xa0
[ 4417.280697]  [<ffffffff8116c2c7>] ? mempool_free_slab+0x17/0x20
[ 4417.282019]  [<ffffffff8116c6c9>] ? mempool_free+0x49/0x90
[ 4417.283326]  [<ffffffff812c9acf>] generic_make_request_checks+0x16f/0x360
[ 4417.284637]  [<ffffffffa0340d97>] ? thin_map+0x227/0x2c0 [dm_thin_pool]
[ 4417.285951]  [<ffffffff812c9ce7>] generic_make_request+0x27/0x130
[ 4417.287240]  [<ffffffffa0230b3d>] __map_bio+0xad/0x100 [dm_mod]
[ 4417.288503]  [<ffffffffa023257e>] __clone_and_map_data_bio+0x15e/0x240 [dm_mod]
[ 4417.289778]  [<ffffffffa02329ea>] __split_and_process_bio+0x38a/0x500 [dm_mod]
[ 4417.291062]  [<ffffffffa0232c91>] dm_make_request+0x131/0x1a0 [dm_mod]
[ 4417.292344]  [<ffffffff812c9da2>] generic_make_request+0xe2/0x130
[ 4417.293626]  [<ffffffff812c9e61>] submit_bio+0x71/0x150
[ 4417.294909]  [<ffffffff8121ab1d>] ? bio_alloc_bioset+0x20d/0x360
[ 4417.296195]  [<ffffffff81215acb>] _submit_bh+0x14b/0x220
[ 4417.297484]  [<ffffffff81215bb0>] submit_bh+0x10/0x20
[ 4417.298744]  [<ffffffffa016d8d8>] jbd2_journal_commit_transaction+0x6c8/0x19a0 [jbd2]
[ 4417.300014]  [<ffffffff810135b8>] ? __switch_to+0xf8/0x4c0
[ 4417.301268]  [<ffffffffa01731e9>] kjournald2+0xc9/0x270 [jbd2]
[ 4417.302524]  [<ffffffff810a0fd0>] ? wake_up_atomic_t+0x30/0x30
[ 4417.303753]  [<ffffffffa0173120>] ? commit_timeout+0x10/0x10 [jbd2]
[ 4417.304950]  [<ffffffff8109ffef>] kthread+0xcf/0xe0
[ 4417.306107]  [<ffffffff8109ff20>] ? kthread_create_on_node+0x140/0x140
[ 4417.307255]  [<ffffffff81647f18>] ret_from_fork+0x58/0x90
[ 4417.308349]  [<ffffffff8109ff20>] ? 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: NJoseph Qi <joseph.qi@linux.alibaba.com>
Reviewed-by: NJiufei Xue <jiufei.xue@linux.alibaba.com>
Reviewed-by: NXiaoguang Wang <xiaoguang.wang@linux.alibaba.com>
Acked-by: NCaspar Zhang <caspar@linux.alibaba.com>
上级 65e6966a
...@@ -1088,6 +1088,10 @@ static void throtl_bio_end_io(struct bio *bio) ...@@ -1088,6 +1088,10 @@ static void throtl_bio_end_io(struct bio *bio)
struct throtl_grp *tg; struct throtl_grp *tg;
rcu_read_lock(); 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; tg = (struct throtl_grp *)bio->bi_tg_private;
if (!tg) if (!tg)
goto out; goto out;
...@@ -1096,6 +1100,7 @@ static void throtl_bio_end_io(struct bio *bio) ...@@ -1096,6 +1100,7 @@ static void throtl_bio_end_io(struct bio *bio)
bio_io_start_time_ns(bio), bio_io_start_time_ns(bio),
bio_op(bio)); bio_op(bio));
blkg_put(tg_to_blkg(tg)); blkg_put(tg_to_blkg(tg));
bio_clear_flag(bio, BIO_THROTL_STATED);
out: out:
rcu_read_unlock(); rcu_read_unlock();
} }
...@@ -1104,11 +1109,19 @@ static inline void throtl_bio_stats_start(struct bio *bio, struct throtl_grp *tg ...@@ -1104,11 +1109,19 @@ static inline void throtl_bio_stats_start(struct bio *bio, struct throtl_grp *tg
{ {
int op = bio_op(bio); 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_end_io = throtl_bio_end_io;
bio->bi_tg_private = tg; bio->bi_tg_private = tg;
bio_set_start_time_ns(bio); bio_set_start_time_ns(bio);
blkg_get(tg_to_blkg(tg));
} }
} }
......
...@@ -238,6 +238,7 @@ struct bio { ...@@ -238,6 +238,7 @@ struct bio {
#define BIO_TRACE_COMPLETION 10 /* bio_endio() should trace the final completion #define BIO_TRACE_COMPLETION 10 /* bio_endio() should trace the final completion
* of this bio. */ * of this bio. */
#define BIO_QUEUE_ENTERED 11 /* can use blk_queue_enter_live() */ #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 */ /* See BVEC_POOL_OFFSET below before adding new flags */
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册