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@xxxxxxxxxxxxxxx # 5.5
> Cc: stable@xxxxxxxxxxxxxxx # 5.4
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx>
Looks good to me.
Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxx>
> ---
> 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))
>
--
Damien Le Moal
Western Digital Research
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
