linux-kernel - Re: [PATCH 2/2] zram: fix uninitialized ZRAM not releasing backing device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241205070939.GF16709@google.com>
Date: Thu, 5 Dec 2024 16:09:39 +0900
From: Sergey Senozhatsky <senozhatsky@...omium.org>
To: Kairui Song <kasong@...cent.com>
Cc: linux-mm@...ck.org, Minchan Kim <minchan@...nel.org>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
	Desheng Wu <deshengwu@...cent.com>, stable@...r.kernel.org
Subject: Re: [PATCH 2/2] zram: fix uninitialized ZRAM not releasing backing
 device

On (24/12/05 02:02), Kairui Song wrote:
> From: Kairui Song <kasong@...cent.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@...cent.com>
> Signed-off-by: Kairui Song <kasong@...cent.com>
> Cc: stable@...r.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);