zram: avoid race between zram_remove and disksize_store

Hello Ming Lei,

The patch 5a4b653655d5: "zram: avoid race between zram_remove and disksize_store" from Oct 25, 2021, leads to the following Smatch static checker warning:

	drivers/block/zram/zram_drv.c:2044 zram_remove()
	warn: 'zram->mem_pool' double freed

drivers/block/zram/zram_drv.c

    2002 static int zram_remove(struct zram *zram)
    2003 {
    2004         struct block_device *bdev = zram->disk->part0;
    2005         bool claimed;
    2006 
    2007         mutex_lock(&bdev->bd_disk->open_mutex);
    2008         if (bdev->bd_openers) {
    2009                 mutex_unlock(&bdev->bd_disk->open_mutex);
    2010                 return -EBUSY;
    2011         }
    2012 
    2013         claimed = zram->claim;
    2014         if (!claimed)
    2015                 zram->claim = true;
    2016         mutex_unlock(&bdev->bd_disk->open_mutex);
    2017 
    2018         zram_debugfs_unregister(zram);
    2019 
    2020         if (claimed) {
    2021                 /*
    2022                  * If we were claimed by reset_store(), del_gendisk() will
    2023                  * wait until reset_store() is done, so nothing need to do.
    2024                  */
    2025                 ;
    2026         } else {
    2027                 /* Make sure all the pending I/O are finished */
    2028                 sync_blockdev(bdev);
    2029                 zram_reset_device(zram);
                         ^^^^^^^^^^^^^^^^^^^^^^^^

This frees zram->mem_pool in zram_meta_free().

    2030         }
    2031 
    2032         pr_info("Removed device: %s\n", zram->disk->disk_name);
    2033 
    2034         del_gendisk(zram->disk);
    2035 
    2036         /* del_gendisk drains pending reset_store */
    2037         WARN_ON_ONCE(claimed && zram->claim);
    2038 
    2039         /*
    2040          * disksize_store() may be called in between zram_reset_device()
    2041          * and del_gendisk(), so run the last reset to avoid leaking
    2042          * anything allocated with disksize_store()
    2043          */

--> 2044 zram_reset_device(zram);

This double frees it.

Hello Ming Lei,

The patch 5a4b653655d5: "zram: avoid race between zram_remove and disksize_store" from Oct 25, 2021, leads to the following Smatch static checker warning:

	drivers/block/zram/zram_drv.c:2044 zram_remove()
	warn: 'zram->mem_pool' double freed

drivers/block/zram/zram_drv.c

    2002 static int zram_remove(struct zram *zram)
    2003 {
    2004         struct block_device *bdev = zram->disk->part0;
    2005         bool claimed;
    2006 
    2007         mutex_lock(&bdev->bd_disk->open_mutex);
    2008         if (bdev->bd_openers) {
    2009                 mutex_unlock(&bdev->bd_disk->open_mutex);
    2010                 return -EBUSY;
    2011         }
    2012 
    2013         claimed = zram->claim;
    2014         if (!claimed)
    2015                 zram->claim = true;
    2016         mutex_unlock(&bdev->bd_disk->open_mutex);
    2017 
    2018         zram_debugfs_unregister(zram);
    2019 
    2020         if (claimed) {
    2021                 /*
    2022                  * If we were claimed by reset_store(), del_gendisk() will
    2023                  * wait until reset_store() is done, so nothing need to do.
    2024                  */
    2025                 ;
    2026         } else {
    2027                 /* Make sure all the pending I/O are finished */
    2028                 sync_blockdev(bdev);
    2029                 zram_reset_device(zram);
                         ^^^^^^^^^^^^^^^^^^^^^^^^

This frees zram->mem_pool in zram_meta_free().

    2030         }
    2031 
    2032         pr_info("Removed device: %s\n", zram->disk->disk_name);
    2033 
    2034         del_gendisk(zram->disk);
    2035 
    2036         /* del_gendisk drains pending reset_store */
    2037         WARN_ON_ONCE(claimed && zram->claim);
    2038 
    2039         /*
    2040          * disksize_store() may be called in between zram_reset_device()
    2041          * and del_gendisk(), so run the last reset to avoid leaking
    2042          * anything allocated with disksize_store()
    2043          */

--> 2044 zram_reset_device(zram);

This double frees it.

Quoted 3 lines

--> 2044 zram_reset_device(zram);

This double frees it.

No.

Inside zram_reset_device(), if init_done()(zram->disksize) is zero, zram_reset_device() returns immediately, otherwise zram->disksize is cleared and zram_meta_free() is run in zram_reset_device(). Meantime zram->init_lock protects the reset and disksize_store().

The 2nd zram_reset_device() can only reset device if disksize_store() sets new zram->disksize and allocates new meta after the 1st zram_reset_device().

Seems smatch static checker need to be improved to cover this case?