Re: [PATCH] zram: fix NULL pointer dereference in zcomp_available_show()

[Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx>]

On (25/08/03 02:25), Seyediman Seyedarab wrote:
> During zram_reset_device(), comp_algs[prio] is set to NULL by
> zram_destroy_comps() before being reinitialized to the default algorithm.
> A concurrent sysfs read can occur between these operations, passing NULL
> to strcmp() and causing a crash.

zram_reset_device() is called under down_write(&zram->init_lock),
while sysfs reads are called under down_read(&zram->init_lock).
zram_reset_device() doesn't release the write ->init_lock until
default_compressor is set.

I think it may make sense to make sure that show() handlers don't
release the read ->init_lock somewhere in between. I only see one
that does so: recomp_algorithm_show().

---

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 8acad3cc6e6e..ee3aa9cc8595 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1228,13 +1228,7 @@ static void comp_algorithm_set(struct zram *zram, u32 prio, const char *alg)
 static ssize_t __comp_algorithm_show(struct zram *zram, u32 prio,
 				     char *buf, ssize_t at)
 {
-	ssize_t sz;
-
-	down_read(&zram->init_lock);
-	sz = zcomp_available_show(zram->comp_algs[prio], buf, at);
-	up_read(&zram->init_lock);
-
-	return sz;
+	return zcomp_available_show(zram->comp_algs[prio], buf, at);
 }
 
 static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
@@ -1387,8 +1381,12 @@ static ssize_t comp_algorithm_show(struct device *dev,
 				   char *buf)
 {
 	struct zram *zram = dev_to_zram(dev);
+	ssize_t sz;
 
-	return __comp_algorithm_show(zram, ZRAM_PRIMARY_COMP, buf, 0);
+	down_read(&zram->init_lock);
+	sz = __comp_algorithm_show(zram, ZRAM_PRIMARY_COMP, buf, 0);
+	up_read(&zram->init_lock);
+	return sz;
 }
 
 static ssize_t comp_algorithm_store(struct device *dev,
@@ -1412,6 +1410,7 @@ static ssize_t recomp_algorithm_show(struct device *dev,
 	ssize_t sz = 0;
 	u32 prio;
 
+	down_read(&zram->init_lock);
 	for (prio = ZRAM_SECONDARY_COMP; prio < ZRAM_MAX_COMPS; prio++) {
 		if (!zram->comp_algs[prio])
 			continue;
@@ -1419,7 +1418,7 @@ static ssize_t recomp_algorithm_show(struct device *dev,
 		sz += sysfs_emit_at(buf, sz, "#%d: ", prio);
 		sz += __comp_algorithm_show(zram, prio, buf, sz);
 	}
-
+	up_read(&zram->init_lock);
 	return sz;
 }