linux-kernel - Re: [PATCH] zram: fix umount-reset

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: <20150203145200.GA1046@swordfish>
Date:	Tue, 3 Feb 2015 23:52:00 +0900
From:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
To:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Cc:	Ganesh Mahendran <opensource.ganesh@...il.com>,
	sergey.senozhatsky.work@...il.com,
	Minchan Kim <minchan@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jerome Marchand <jmarchan@...hat.com>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] zram: fix umount-reset_store-mount race condition

On (02/03/15 23:15), Sergey Senozhatsky wrote:
> > How about keep this here? Protected by zram->init_lock.
> >            set_capacity(zram->disk, 0);
> 
> why?
> 
yeah, I see why. good catch.

hm, why do we perform destroy_device() before zram_reset_device() in
zram_exit()?

how about doing something like this (I don't want to return 
that bool param back):

---

 drivers/block/zram/zram_drv.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index a32069f..386f7ed 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -736,6 +736,7 @@ static void zram_reset_device(struct zram *zram)
 	memset(&zram->stats, 0, sizeof(zram->stats));
 
 	zram->disksize = 0;
+	set_capacity(zram->disk, 0);
 	up_write(&zram->init_lock);
 }
 
@@ -828,7 +829,6 @@ static ssize_t reset_store(struct device *dev,
 	/* Make sure all pending I/O is finished */
 	fsync_bdev(bdev);
 	zram_reset_device(zram);
-	set_capacity(zram->disk, 0);
 
 	mutex_unlock(&bdev->bd_mutex);
 	revalidate_disk(zram->disk);
@@ -1178,12 +1178,8 @@ static void __exit zram_exit(void)
 	for (i = 0; i < num_devices; i++) {
 		zram = &zram_devices[i];
 
-		destroy_device(zram);
-		/*
-		 * Shouldn't access zram->disk after destroy_device
-		 * because destroy_device already released zram->disk.
-		 */
 		zram_reset_device(zram);
+		destroy_device(zram);
 	}
 
 	unregister_blkdev(zram_major, "zram");

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/