| Message ID | 20200227001852.287194-1-shinichiro.kawasaki@wdc.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
| Series | dm zoned: Fix reference counter initial value of chunk works | expand |
On 2020/02/27 9:18, Shin'ichiro Kawasaki wrote: > Dm-zoned initializes reference counters of new chunk works with zero > value and refcount_inc() is called to increment the counter. However, the > refcount_inc() function handles the addition to zero value as an error > and triggers the warning as follows: > > refcount_t: addition on 0; use-after-free. > WARNING: CPU: 7 PID: 1506 at lib/refcount.c:25 refcount_warn_saturate+0x68/0xf0 > Modules linked in: dm_zoned bridge stp llc rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache nf_conntrack_netbios_ns nf_o > CPU: 7 PID: 1506 Comm: systemd-udevd Not tainted 5.4.0+ #134 > ... > Call Trace: > dmz_map+0x2d2/0x350 [dm_zoned] > __map_bio+0x42/0x1a0 > __split_and_process_non_flush+0x14a/0x1b0 > __split_and_process_bio+0x83/0x240 > ? kmem_cache_alloc+0x165/0x220 > dm_process_bio+0x90/0x230 > ? generic_make_request_checks+0x2e7/0x680 > dm_make_request+0x3e/0xb0 > generic_make_request+0xcf/0x320 > ? memcg_drain_all_list_lrus+0x1c0/0x1c0 > submit_bio+0x3c/0x160 > ? guard_bio_eod+0x2c/0x130 > mpage_readpages+0x182/0x1d0 > ? bdev_evict_inode+0xf0/0xf0 > read_pages+0x6b/0x1b0 > __do_page_cache_readahead+0x1ba/0x1d0 > force_page_cache_readahead+0x93/0x100 > generic_file_read_iter+0x83a/0xe40 > ? __seccomp_filter+0x7b/0x670 > new_sync_read+0x12a/0x1c0 > vfs_read+0x9d/0x150 > ksys_read+0x5f/0xe0 > do_syscall_64+0x5b/0x180 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > ... > > After this warning, following refcount API calls for the counter all fail > to change the counter value. > > Fix this by setting the initial reference counter value not zero but one > for the new chunk works. Instead, do not call refcount_inc() via > dmz_get_chunk_work() for the new chunks works. > > The failure was observed with linux version 5.4 with CONFIG_REFCOUNT_FULL > enabled. Refcount rework was merged to linux version 5.5 by the > commit 168829ad09ca ("Merge branch 'locking-core-for-linus' of > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip"). After this > commit, CONFIG_REFCOUNT_FULL was removed and the failure was observed > regardless of kernel configuration. > > Linux version 4.20 merged the commit 092b5648760a ("dm zoned: target: use > refcount_t for dm zoned reference counters"). Before this commit, dm > zoned used atomic_t APIs which does not check addition to zero, then this > fix is not necessary. > > Fixes: 092b5648760a ("dm zoned: target: use refcount_t for dm zoned reference counters") > Cc: stable@vger.kernel.org # 5.5 > Cc: stable@vger.kernel.org # 5.4 > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> Looks good to me. Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com> > --- > drivers/md/dm-zoned-target.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c > index 70a1063161c0..b1e64cd31647 100644 > --- a/drivers/md/dm-zoned-target.c > +++ b/drivers/md/dm-zoned-target.c > @@ -533,8 +533,9 @@ static int dmz_queue_chunk_work(struct dmz_target *dmz, struct bio *bio) > > /* Get the BIO chunk work. If one is not active yet, create one */ > cw = radix_tree_lookup(&dmz->chunk_rxtree, chunk); > - if (!cw) { > - > + if (cw) { > + dmz_get_chunk_work(cw); > + } else { > /* Create a new chunk work */ > cw = kmalloc(sizeof(struct dm_chunk_work), GFP_NOIO); > if (unlikely(!cw)) { > @@ -543,7 +544,7 @@ static int dmz_queue_chunk_work(struct dmz_target *dmz, struct bio *bio) > } > > INIT_WORK(&cw->work, dmz_chunk_work); > - refcount_set(&cw->refcount, 0); > + refcount_set(&cw->refcount, 1); > cw->target = dmz; > cw->chunk = chunk; > bio_list_init(&cw->bio_list); > @@ -556,7 +557,6 @@ static int dmz_queue_chunk_work(struct dmz_target *dmz, struct bio *bio) > } > > bio_list_add(&cw->bio_list, bio); > - dmz_get_chunk_work(cw); > > dmz_reclaim_bio_acc(dmz->reclaim); > if (queue_work(dmz->chunk_wq, &cw->work)) >
diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c index 70a1063161c0..b1e64cd31647 100644 --- a/drivers/md/dm-zoned-target.c +++ b/drivers/md/dm-zoned-target.c @@ -533,8 +533,9 @@ static int dmz_queue_chunk_work(struct dmz_target *dmz, struct bio *bio) /* Get the BIO chunk work. If one is not active yet, create one */ cw = radix_tree_lookup(&dmz->chunk_rxtree, chunk); - if (!cw) { - + if (cw) { + dmz_get_chunk_work(cw); + } else { /* Create a new chunk work */ cw = kmalloc(sizeof(struct dm_chunk_work), GFP_NOIO); if (unlikely(!cw)) { @@ -543,7 +544,7 @@ static int dmz_queue_chunk_work(struct dmz_target *dmz, struct bio *bio) } INIT_WORK(&cw->work, dmz_chunk_work); - refcount_set(&cw->refcount, 0); + refcount_set(&cw->refcount, 1); cw->target = dmz; cw->chunk = chunk; bio_list_init(&cw->bio_list); @@ -556,7 +557,6 @@ static int dmz_queue_chunk_work(struct dmz_target *dmz, struct bio *bio) } bio_list_add(&cw->bio_list, bio); - dmz_get_chunk_work(cw); dmz_reclaim_bio_acc(dmz->reclaim); if (queue_work(dmz->chunk_wq, &cw->work))
Dm-zoned initializes reference counters of new chunk works with zero value and refcount_inc() is called to increment the counter. However, the refcount_inc() function handles the addition to zero value as an error and triggers the warning as follows: refcount_t: addition on 0; use-after-free. WARNING: CPU: 7 PID: 1506 at lib/refcount.c:25 refcount_warn_saturate+0x68/0xf0 Modules linked in: dm_zoned bridge stp llc rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache nf_conntrack_netbios_ns nf_o CPU: 7 PID: 1506 Comm: systemd-udevd Not tainted 5.4.0+ #134 ... Call Trace: dmz_map+0x2d2/0x350 [dm_zoned] __map_bio+0x42/0x1a0 __split_and_process_non_flush+0x14a/0x1b0 __split_and_process_bio+0x83/0x240 ? kmem_cache_alloc+0x165/0x220 dm_process_bio+0x90/0x230 ? generic_make_request_checks+0x2e7/0x680 dm_make_request+0x3e/0xb0 generic_make_request+0xcf/0x320 ? memcg_drain_all_list_lrus+0x1c0/0x1c0 submit_bio+0x3c/0x160 ? guard_bio_eod+0x2c/0x130 mpage_readpages+0x182/0x1d0 ? bdev_evict_inode+0xf0/0xf0 read_pages+0x6b/0x1b0 __do_page_cache_readahead+0x1ba/0x1d0 force_page_cache_readahead+0x93/0x100 generic_file_read_iter+0x83a/0xe40 ? __seccomp_filter+0x7b/0x670 new_sync_read+0x12a/0x1c0 vfs_read+0x9d/0x150 ksys_read+0x5f/0xe0 do_syscall_64+0x5b/0x180 entry_SYSCALL_64_after_hwframe+0x44/0xa9 ... After this warning, following refcount API calls for the counter all fail to change the counter value. Fix this by setting the initial reference counter value not zero but one for the new chunk works. Instead, do not call refcount_inc() via dmz_get_chunk_work() for the new chunks works. The failure was observed with linux version 5.4 with CONFIG_REFCOUNT_FULL enabled. Refcount rework was merged to linux version 5.5 by the commit 168829ad09ca ("Merge branch 'locking-core-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip"). After this commit, CONFIG_REFCOUNT_FULL was removed and the failure was observed regardless of kernel configuration. Linux version 4.20 merged the commit 092b5648760a ("dm zoned: target: use refcount_t for dm zoned reference counters"). Before this commit, dm zoned used atomic_t APIs which does not check addition to zero, then this fix is not necessary. Fixes: 092b5648760a ("dm zoned: target: use refcount_t for dm zoned reference counters") Cc: stable@vger.kernel.org # 5.5 Cc: stable@vger.kernel.org # 5.4 Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> --- drivers/md/dm-zoned-target.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)