1

dm cache: fix flushing uninitialized delayed_work on cache_ctr error

An unexpected WARN_ON from flush_work() may occur when cache creation
fails, caused by destroying the uninitialized delayed_work waker in the
error path of cache_create(). For example, the warning appears on the
superblock checksum error.

Reproduce steps:

dmsetup create cmeta --table "0 8192 linear /dev/sdc 0"
dmsetup create cdata --table "0 65536 linear /dev/sdc 8192"
dmsetup create corig --table "0 524288 linear /dev/sdc 262144"
dd if=/dev/urandom of=/dev/mapper/cmeta bs=4k count=1 oflag=direct
dmsetup create cache --table "0 524288 cache /dev/mapper/cmeta \
/dev/mapper/cdata /dev/mapper/corig 128 2 metadata2 writethrough smq 0"

Kernel logs:

(snip)
WARNING: CPU: 0 PID: 84 at kernel/workqueue.c:4178 __flush_work+0x5d4/0x890

Fix by pulling out the cancel_delayed_work_sync() from the constructor's
error path. This patch doesn't affect the use-after-free fix for
concurrent dm_resume and dm_destroy (commit 6a459d8edb ("dm cache: Fix
UAF in destroy()")) as cache_dtr is not changed.

Signed-off-by: Ming-Hung Tsai <mtsai@redhat.com>
Fixes: 6a459d8edb ("dm cache: Fix UAF in destroy()")
Cc: stable@vger.kernel.org
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Acked-by: Joe Thornber <thornber@redhat.com>
This commit is contained in:
Ming-Hung Tsai 2024-10-22 15:12:49 +08:00 committed by Mikulas Patocka
parent 235d2e739f
commit 135496c208

View File

@ -1905,16 +1905,13 @@ static void check_migrations(struct work_struct *ws)
* This function gets called on the error paths of the constructor, so we * This function gets called on the error paths of the constructor, so we
* have to cope with a partially initialised struct. * have to cope with a partially initialised struct.
*/ */
static void destroy(struct cache *cache) static void __destroy(struct cache *cache)
{ {
unsigned int i;
mempool_exit(&cache->migration_pool); mempool_exit(&cache->migration_pool);
if (cache->prison) if (cache->prison)
dm_bio_prison_destroy_v2(cache->prison); dm_bio_prison_destroy_v2(cache->prison);
cancel_delayed_work_sync(&cache->waker);
if (cache->wq) if (cache->wq)
destroy_workqueue(cache->wq); destroy_workqueue(cache->wq);
@ -1942,13 +1939,22 @@ static void destroy(struct cache *cache)
if (cache->policy) if (cache->policy)
dm_cache_policy_destroy(cache->policy); dm_cache_policy_destroy(cache->policy);
bioset_exit(&cache->bs);
kfree(cache);
}
static void destroy(struct cache *cache)
{
unsigned int i;
cancel_delayed_work_sync(&cache->waker);
for (i = 0; i < cache->nr_ctr_args ; i++) for (i = 0; i < cache->nr_ctr_args ; i++)
kfree(cache->ctr_args[i]); kfree(cache->ctr_args[i]);
kfree(cache->ctr_args); kfree(cache->ctr_args);
bioset_exit(&cache->bs); __destroy(cache);
kfree(cache);
} }
static void cache_dtr(struct dm_target *ti) static void cache_dtr(struct dm_target *ti)
@ -2561,7 +2567,7 @@ static int cache_create(struct cache_args *ca, struct cache **result)
*result = cache; *result = cache;
return 0; return 0;
bad: bad:
destroy(cache); __destroy(cache);
return r; return r;
} }
@ -2612,7 +2618,7 @@ static int cache_ctr(struct dm_target *ti, unsigned int argc, char **argv)
r = copy_ctr_args(cache, argc - 3, (const char **)argv + 3); r = copy_ctr_args(cache, argc - 3, (const char **)argv + 3);
if (r) { if (r) {
destroy(cache); __destroy(cache);
goto out; goto out;
} }