[PATCH] zram: do not let user enforce new device dev_id

[Sergey Senozhatsky]

This patch forbids user to enforce device ids for newly added zram devices,
as suggested by Minchan Kim. There seems to be a little interest in this
functionality and its use-cases are rather non-obvious.

zram_add sysfs attr, thus, is now read only and has only automatic device
id assignment mode.

This operation is no longer allowed:
   echo 1 > /sys/class/zram-control/zram_add
   -bash: /sys/class/zram-control/zram_add: Permission denied

Correct usage example:
   cat /sys/class/zram-control/zram_add
   2

It also removes common zram_control() handler because device creation and
removal do not share a lot of common steps anymore. Move zram_add and
zram_remove code to zram_add_show() and zram_remove_store() correspondingly.

LKML-reference: https://lkml.org/lkml/2015/3/4/1414
Reported-by: Minchan Kim <minchan@xxxxxxxxxx>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@xxxxxxxxx>
---
 Documentation/ABI/testing/sysfs-class-zram |   8 +-
 Documentation/blockdev/zram.txt            |  20 ++---
 drivers/block/zram/zram_drv.c              | 130 +++++++++++------------------
 3 files changed, 59 insertions(+), 99 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-zram b/Documentation/ABI/testing/sysfs-class-zram
index 0b678b2..c39d287 100644
--- a/Documentation/ABI/testing/sysfs-class-zram
+++ b/Documentation/ABI/testing/sysfs-class-zram
@@ -11,11 +11,9 @@ Date:		March 2015
 KernelVersion:	4.1
 Contact:	Sergey Senozhatsky <sergey.senozhatsky@xxxxxxxxx>
 Description:
-		RW attribuite. Write operation adds a specific /dev/zramX
-		device, where X is a device_id provided by user.
-		Read operation will automatically pick up avilable device_id
-		X, add /dev/zramX device and return that device_id X back to
-		user.
+		RO attribute. Read operation will cause zram to add a new
+		device and return its device id back to user (so one can
+		use /dev/zram<id>), or error code.
 
 		What:		/sys/class/zram-control/zram_add
 Date:		March 2015
diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index 6bbdded..902c97c 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -104,23 +104,17 @@ size of the disk when not in use so a huge zram is wasteful.
 zram provides a control interface, which enables dynamic (on-demand) device
 addition and removal.
 
-In order to add a new /dev/zramX device (where X is a unique device id)
-execute
-	echo X > /sys/class/zram-control/zram_add
-
-To remove the existing /dev/zramX device (where X is a device id)
-execute
-	echo X > /sys/class/zram-control/zram_remove
-
-Additionally, zram also handles automatic device_id generation and assignment.
+In order to add a new /dev/zramX device, perform read operation on zram_add
+attribute. This will return either new device's device id (meaning that you
+can use /dev/zram<id>) or error code.
 
+Example:
 	cat /sys/class/zram-control/zram_add
 	1
-	cat /sys/class/zram-control/zram_add
-	2
 
-this will pick up available device_id X, add corresponding /dev/zramX
-device and return its device_id X back to user (or error code).
+To remove the existing /dev/zramX device (where X is a device id)
+execute
+	echo X > /sys/class/zram-control/zram_remove
 
 8) Stats:
 	Per-device statistics are exported as various nodes under
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 6d27518..0194799 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -47,9 +47,6 @@ static const char *default_compressor = "lzo";
 /* Module params (documentation at end) */
 static unsigned int num_devices = 1;
 
-#define ZRAM_CTL_ADD		1
-#define ZRAM_CTL_REMOVE		2
-
 #define ZRAM_ATTR_RO(name)						\
 static ssize_t name##_show(struct device *d,				\
 				struct device_attribute *attr, char *b)	\
@@ -1230,79 +1227,7 @@ static struct zram *zram_lookup(int dev_id)
 	return ERR_PTR(-ENODEV);
 }
 
-/* common zram-control add/remove handler */
-static int zram_control(int cmd, const char *buf)
-{
-	struct zram *zram;
-	int ret = -ENOSYS, err, dev_id;
-
-	/* dev_id is gendisk->first_minor, which is `int' */
-	ret = kstrtoint(buf, 10, &dev_id);
-	if (ret || dev_id < 0)
-		return ret;
-
-	mutex_lock(&zram_index_mutex);
-	zram = zram_lookup(dev_id);
-
-	switch (cmd) {
-	case ZRAM_CTL_ADD:
-		if (!IS_ERR(zram)) {
-			ret = -EEXIST;
-			break;
-		}
-		ret = zram_add(dev_id);
-		break;
-	case ZRAM_CTL_REMOVE:
-		if (IS_ERR(zram)) {
-			ret = PTR_ERR(zram);
-			break;
-		}
-
-		/*
-		 * First, make ->disksize device attr RO, closing
-		 * ZRAM_CTL_REMOVE vs disksize_store() race window
-		 */
-		ret = sysfs_chmod_file(&disk_to_dev(zram->disk)->kobj,
-				&dev_attr_disksize.attr, S_IRUGO);
-		if (ret)
-			break;
-
-		ret = zram_reset_device(zram);
-		if (ret == 0) {
-			/* ->disksize is RO and there are no ->bd_openers */
-			zram_remove(zram);
-			break;
-		}
-
-		/*
-		 * If there are still device bd_openers, try to make ->disksize
-		 * RW again and return. even if we fail to make ->disksize RW,
-		 * user still has RW ->reset attr. so it's possible to destroy
-		 * that device.
-		 */
-		err = sysfs_chmod_file(&disk_to_dev(zram->disk)->kobj,
-				&dev_attr_disksize.attr,
-				S_IWUSR | S_IRUGO);
-		if (err)
-			ret = err;
-		break;
-	}
-	mutex_unlock(&zram_index_mutex);
-
-	return ret;
-}
-
 /* zram module control sysfs attributes */
-static ssize_t zram_add_store(struct class *class,
-			struct class_attribute *attr,
-			const char *buf,
-			size_t count)
-{
-	int ret = zram_control(ZRAM_CTL_ADD, buf);
-
-	return ret ? ret : count;
-}
-
 static ssize_t zram_add_show(struct class *class,
 			struct class_attribute *attr,
 			char *buf)
@@ -1310,10 +1235,7 @@ static ssize_t zram_add_show(struct class *class,
 	int ret;
 
 	mutex_lock(&zram_index_mutex);
-	/*
-	 * read operation on zram_add is - pick up device_id automatically, add
-	 * corresponding device and return that device_id back to user
-	 */
+	/* pick up available device_id */
 	ret = zram_add(-1);
 	mutex_unlock(&zram_index_mutex);
 
@@ -1327,13 +1249,59 @@ static ssize_t zram_remove_store(struct class *class,
 			const char *buf,
 			size_t count)
 {
-	int ret = zram_control(ZRAM_CTL_REMOVE, buf);
+	struct zram *zram;
+	int ret, err, dev_id;
+
+	mutex_lock(&zram_index_mutex);
+
+	/* dev_id is gendisk->first_minor, which is `int' */
+	ret = kstrtoint(buf, 10, &dev_id);
+	if (ret || dev_id < 0) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	zram = zram_lookup(dev_id);
+	if (IS_ERR(zram)) {
+		ret = PTR_ERR(zram);
+		goto out;
+	}
+
+	/*
+	 * First, make ->disksize device attr RO, closing
+	 * ZRAM_CTL_REMOVE vs disksize_store() race window
+	 */
+	ret = sysfs_chmod_file(&disk_to_dev(zram->disk)->kobj,
+			&dev_attr_disksize.attr, S_IRUGO);
+	if (ret)
+		goto out;
+
+	ret = zram_reset_device(zram);
+	if (ret == 0) {
+		/* ->disksize is RO and there are no ->bd_openers */
+		zram_remove(zram);
+		goto out;
+	}
+
+	/*
+	 * If there are still device bd_openers, try to make ->disksize
+	 * RW again and return. even if we fail to make ->disksize RW,
+	 * user still has RW ->reset attr. so it's possible to destroy
+	 * that device.
+	 */
+	err = sysfs_chmod_file(&disk_to_dev(zram->disk)->kobj,
+			&dev_attr_disksize.attr,
+			S_IWUSR | S_IRUGO);
+	if (err)
+		ret = err;
 
+out:
+	mutex_unlock(&zram_index_mutex);
 	return ret ? ret : count;
 }
 
 static struct class_attribute zram_control_class_attrs[] = {
-	__ATTR_RW(zram_add),
+	__ATTR_RO(zram_add),
 	__ATTR_WO(zram_remove),
 	__ATTR_NULL,
 };
-- 
2.3.1.184.g97c12a8

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