Re: [PATCH 5/5] dm cache: fix potential out-of-bounds access on the first resume

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

 



ack

On Tue, Oct 22, 2024 at 8:14 AM Ming-Hung Tsai <mtsai@xxxxxxxxxx> wrote:
>
> Out-of-bounds access occurs if the fast device is expanded unexpectedly
> before the first-time resume of the cache table. This happens because
> expanding the fast device requires reloading the cache table for
> cache_create to allocate new in-core data structures that fit the new
> size, and the check in cache_preresume is not performed during the
> first resume, leading to the issue.
>
> Reproduce steps:
>
> 1. prepare component devices:
>
> 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/zero of=/dev/mapper/cmeta bs=4k count=1 oflag=direct
>
> 2. load a cache table of 512 cache blocks, and deliberately expand the
>    fast device before resuming the cache, making the in-core data
>    structures inadequate.
>
> dmsetup create cache --notable
> dmsetup reload cache --table "0 524288 cache /dev/mapper/cmeta \
> /dev/mapper/cdata /dev/mapper/corig 128 2 metadata2 writethrough smq 0"
> dmsetup reload cdata --table "0 131072 linear /dev/sdc 8192"
> dmsetup resume cdata
> dmsetup resume cache
>
> 3. suspend the cache to write out the in-core dirty bitset and hint
>    array, leading to out-of-bounds access to the dirty bitset at offset
>    0x40:
>
> dmsetup suspend cache
>
> KASAN reports:
>
>   BUG: KASAN: vmalloc-out-of-bounds in is_dirty_callback+0x2b/0x80
>   Read of size 8 at addr ffffc90000085040 by task dmsetup/90
>
>   (...snip...)
>   The buggy address belongs to the virtual mapping at
>    [ffffc90000085000, ffffc90000087000) created by:
>    cache_ctr+0x176a/0x35f0
>
>   (...snip...)
>   Memory state around the buggy address:
>    ffffc90000084f00: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
>    ffffc90000084f80: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
>   >ffffc90000085000: 00 00 00 00 00 00 00 00 f8 f8 f8 f8 f8 f8 f8 f8
>                                              ^
>    ffffc90000085080: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
>    ffffc90000085100: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
>
> Fix by checking the size change on the first resume.
>
> Signed-off-by: Ming-Hung Tsai <mtsai@xxxxxxxxxx>
> Fixes: f494a9c6b1b6 ("dm cache: cache shrinking support")
> ---
>  drivers/md/dm-cache-target.c | 37 ++++++++++++++++--------------------
>  1 file changed, 16 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
> index fa8ef2c32af8..40709310e327 100644
> --- a/drivers/md/dm-cache-target.c
> +++ b/drivers/md/dm-cache-target.c
> @@ -2901,24 +2901,24 @@ static dm_cblock_t get_cache_dev_size(struct cache *cache)
>  static bool can_resize(struct cache *cache, dm_cblock_t new_size)
>  {
>         if (from_cblock(new_size) > from_cblock(cache->cache_size)) {
> -               if (cache->sized) {
> -                       DMERR("%s: unable to extend cache due to missing cache table reload",
> -                             cache_device_name(cache));
> -                       return false;
> -               }
> +               DMERR("%s: unable to extend cache due to missing cache table reload",
> +                     cache_device_name(cache));
> +               return false;
>         }
>
>         /*
>          * We can't drop a dirty block when shrinking the cache.
>          */
> -       new_size = to_cblock(find_next_bit(cache->dirty_bitset,
> -                                          from_cblock(cache->cache_size),
> -                                          from_cblock(new_size)));
> -       if (new_size != cache->cache_size) {
> -               DMERR("%s: unable to shrink cache; cache block %llu is dirty",
> -                     cache_device_name(cache),
> -                     (unsigned long long) from_cblock(new_size));
> -               return false;
> +       if (cache->loaded_mappings) {
> +               new_size = to_cblock(find_next_bit(cache->dirty_bitset,
> +                                                  from_cblock(cache->cache_size),
> +                                                  from_cblock(new_size)));
> +               if (new_size != cache->cache_size) {
> +                       DMERR("%s: unable to shrink cache; cache block %llu is dirty",
> +                             cache_device_name(cache),
> +                             (unsigned long long) from_cblock(new_size));
> +                       return false;
> +               }
>         }
>
>         return true;
> @@ -2949,20 +2949,15 @@ static int cache_preresume(struct dm_target *ti)
>         /*
>          * Check to see if the cache has resized.
>          */
> -       if (!cache->sized) {
> -               r = resize_cache_dev(cache, csize);
> -               if (r)
> -                       return r;
> -
> -               cache->sized = true;
> -
> -       } else if (csize != cache->cache_size) {
> +       if (!cache->sized || csize != cache->cache_size) {
>                 if (!can_resize(cache, csize))
>                         return -EINVAL;
>
>                 r = resize_cache_dev(cache, csize);
>                 if (r)
>                         return r;
> +
> +               cache->sized = true;
>         }
>
>         if (!cache->loaded_mappings) {
> --
> 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