Re: [PATCH 2/5] dm cache: fix flushing uninitialized delayed_work on cache_ctr error

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



ack

On Tue, Oct 22, 2024 at 8:13 AM Ming-Hung Tsai <mtsai@xxxxxxxxxx> wrote:
>
> 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 6a459d8edbdb ("dm cache: Fix
> UAF in destroy()")) as cache_dtr is not changed.
>
> Signed-off-by: Ming-Hung Tsai <mtsai@xxxxxxxxxx>
> Fixes: 6a459d8edbdb ("dm cache: Fix UAF in destroy()")
> ---
>  drivers/md/dm-cache-target.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
> index 90772b42c234..68e9a1abd303 100644
> --- a/drivers/md/dm-cache-target.c
> +++ b/drivers/md/dm-cache-target.c
> @@ -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
>   * 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);
>
>         if (cache->prison)
>                 dm_bio_prison_destroy_v2(cache->prison);
>
> -       cancel_delayed_work_sync(&cache->waker);
>         if (cache->wq)
>                 destroy_workqueue(cache->wq);
>
> @@ -1942,13 +1939,22 @@ static void destroy(struct cache *cache)
>         if (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++)
>                 kfree(cache->ctr_args[i]);
>         kfree(cache->ctr_args);
>
> -       bioset_exit(&cache->bs);
> -
> -       kfree(cache);
> +       __destroy(cache);
>  }
>
>  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;
>         return 0;
>  bad:
> -       destroy(cache);
> +       __destroy(cache);
>         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);
>         if (r) {
> -               destroy(cache);
> +               __destroy(cache);
>                 goto out;
>         }
>
> --
> 2.47.0
>






[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux