From: Kairui Song kasong@tencent.com
Setting backing device is done before ZRAM initialization. If we set the backing device, then remove the ZRAM module without initializing the device, the backing device reference will be leaked and the device will be hold forever.
Fix this by always check and release the backing device when resetting or removing ZRAM.
Fixes: 013bf95a83ec ("zram: add interface to specif backing device") Reported-by: Desheng Wu deshengwu@tencent.com Signed-off-by: Kairui Song kasong@tencent.com Cc: stable@vger.kernel.org --- drivers/block/zram/zram_drv.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index dd48df5b97c8..dfe9a994e437 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -2335,6 +2335,9 @@ static void zram_reset_device(struct zram *zram) zram->limit_pages = 0;
if (!init_done(zram)) { + /* Backing device could be set before ZRAM initialization. */ + reset_bdev(zram); + up_write(&zram->init_lock); return; }
On (24/12/05 02:02), Kairui Song wrote:
From: Kairui Song kasong@tencent.com
Setting backing device is done before ZRAM initialization. If we set the backing device, then remove the ZRAM module without initializing the device, the backing device reference will be leaked and the device will be hold forever.
Fix this by always check and release the backing device when resetting or removing ZRAM.
Fixes: 013bf95a83ec ("zram: add interface to specif backing device") Reported-by: Desheng Wu deshengwu@tencent.com Signed-off-by: Kairui Song kasong@tencent.com Cc: stable@vger.kernel.org
drivers/block/zram/zram_drv.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index dd48df5b97c8..dfe9a994e437 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -2335,6 +2335,9 @@ static void zram_reset_device(struct zram *zram) zram->limit_pages = 0; if (!init_done(zram)) {
/* Backing device could be set before ZRAM initialization. */
reset_bdev(zram);
- up_write(&zram->init_lock); return; }
--
So here I think we better remove that if entirely and always reset the device. Something like this (untested):
---
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 0ca6d55c9917..8773b12afc9d 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -1438,12 +1438,16 @@ static void zram_meta_free(struct zram *zram, u64 disksize) size_t num_pages = disksize >> PAGE_SHIFT; size_t index;
+ if (!zram->table) + return; + /* Free all pages that are still in this zram device */ for (index = 0; index < num_pages; index++) zram_free_page(zram, index);
zs_destroy_pool(zram->mem_pool); vfree(zram->table); + zram->table = NULL; }
static bool zram_meta_alloc(struct zram *zram, u64 disksize) @@ -2327,12 +2331,6 @@ static void zram_reset_device(struct zram *zram) down_write(&zram->init_lock);
zram->limit_pages = 0; - - if (!init_done(zram)) { - up_write(&zram->init_lock); - return; - } - set_capacity_and_notify(zram->disk, 0); part_stat_set_all(zram->disk->part0, 0);
On Thu, Dec 5, 2024 at 3:09 PM Sergey Senozhatsky senozhatsky@chromium.org wrote:
On (24/12/05 02:02), Kairui Song wrote:
From: Kairui Song kasong@tencent.com
Setting backing device is done before ZRAM initialization. If we set the backing device, then remove the ZRAM module without initializing the device, the backing device reference will be leaked and the device will be hold forever.
Fix this by always check and release the backing device when resetting or removing ZRAM.
Fixes: 013bf95a83ec ("zram: add interface to specif backing device") Reported-by: Desheng Wu deshengwu@tencent.com Signed-off-by: Kairui Song kasong@tencent.com Cc: stable@vger.kernel.org
drivers/block/zram/zram_drv.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index dd48df5b97c8..dfe9a994e437 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -2335,6 +2335,9 @@ static void zram_reset_device(struct zram *zram) zram->limit_pages = 0;
if (!init_done(zram)) {
/* Backing device could be set before ZRAM initialization. */
reset_bdev(zram);
up_write(&zram->init_lock); return; }
--
So here I think we better remove that if entirely and always reset the device. Something like this (untested):
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 0ca6d55c9917..8773b12afc9d 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -1438,12 +1438,16 @@ static void zram_meta_free(struct zram *zram, u64 disksize) size_t num_pages = disksize >> PAGE_SHIFT; size_t index;
if (!zram->table)
return;
/* Free all pages that are still in this zram device */ for (index = 0; index < num_pages; index++) zram_free_page(zram, index); zs_destroy_pool(zram->mem_pool); vfree(zram->table);
zram->table = NULL;
}
static bool zram_meta_alloc(struct zram *zram, u64 disksize) @@ -2327,12 +2331,6 @@ static void zram_reset_device(struct zram *zram) down_write(&zram->init_lock);
zram->limit_pages = 0;
if (!init_done(zram)) {
up_write(&zram->init_lock);
return;
}
set_capacity_and_notify(zram->disk, 0); part_stat_set_all(zram->disk->part0, 0);
Thanks for the suggestion, I've tested it and it works well. Will send a V2 shortly.
linux-stable-mirror@lists.linaro.org