[PATCH v3 1/3] interconnect: debugfs: replace writable string helper

debugfs_create_str() is being made read-only because its generic write
path is hard to make safe without adding more locking to the helper.

Convert the interconnect debugfs client src_node and dst_node entries to
local file operations before removing writable string support from
debugfs_create_str().  Protect the string replacement and path lookup with
the existing debugfs_lock.

The old code duplicated the strings under rcu_read_lock(), so it had to use
GFP_ATOMIC.  The local file operations protect src_node and dst_node with
debugfs_lock instead, so the allocation can use GFP_KERNEL.

Signed-off-by: Yichong Chen <[email protected]>
---
 drivers/interconnect/debugfs-client.c | 81 +++++++++++++++++++++------
 1 file changed, 64 insertions(+), 17 deletions(-)

diff --git a/drivers/interconnect/debugfs-client.c b/drivers/interconnect/debugfs-client.c
index 08df9188ef94..c587fbf519c6 100644
--- a/drivers/interconnect/debugfs-client.c
+++ b/drivers/interconnect/debugfs-client.c
@@ -5,6 +5,7 @@
 #include <linux/debugfs.h>
 #include <linux/interconnect.h>
 #include <linux/platform_device.h>
+#include <linux/slab.h>
 
 #include "internal.h"
 
@@ -36,6 +37,59 @@ struct debugfs_path {
 	struct list_head list;
 };
 
+static ssize_t icc_node_read(struct file *file, char __user *user_buf,
+			     size_t count, loff_t *ppos)
+{
+	char **node = file->private_data;
+	char *copy;
+	size_t len;
+	ssize_t ret;
+
+	scoped_guard(mutex, &debugfs_lock) {
+		copy = kstrdup(*node ?: "", GFP_KERNEL);
+	}
+	if (!copy)
+		return -ENOMEM;
+
+	len = strlen(copy);
+	copy[len++] = '\n';
+	ret = simple_read_from_buffer(user_buf, count, ppos, copy, len);
+	kfree(copy);
+	return ret;
+}
+
+static ssize_t icc_node_write(struct file *file, const char __user *user_buf,
+			      size_t count, loff_t *ppos)
+{
+	char **node = file->private_data;
+	char *old, *new;
+
+	if (*ppos)
+		return -EINVAL;
+	if (count + 1 > PAGE_SIZE)
+		return -E2BIG;
+
+	new = memdup_user_nul(user_buf, count);
+	if (IS_ERR(new))
+		return PTR_ERR(new);
+	strim(new);
+
+	scoped_guard(mutex, &debugfs_lock) {
+		old = *node;
+		*node = new;
+	}
+
+	kfree(old);
+	return count;
+}
+
+static const struct file_operations icc_node_fops = {
+	.open = simple_open,
+	.read = icc_node_read,
+	.write = icc_node_write,
+	.llseek = default_llseek,
+};
+
 static struct icc_path *get_path(const char *src, const char *dst)
 {
 	struct debugfs_path *path;
@@ -54,26 +108,19 @@ static int icc_get_set(void *data, u64 val)
 	char *src, *dst;
 	int ret = 0;
 
-	mutex_lock(&debugfs_lock);
-
-	rcu_read_lock();
-	src = rcu_dereference(src_node);
-	dst = rcu_dereference(dst_node);
+	guard(mutex)(&debugfs_lock);
 
 	/*
 	 * If we've already looked up a path, then use the existing one instead
 	 * of calling icc_get() again. This allows for updating previous BW
 	 * votes when "get" is written to multiple times for multiple paths.
 	 */
-	cur_path = get_path(src, dst);
-	if (cur_path) {
-		rcu_read_unlock();
+	cur_path = get_path(src_node, dst_node);
+	if (cur_path)
 		goto out;
-	}
 
-	src = kstrdup(src, GFP_ATOMIC);
-	dst = kstrdup(dst, GFP_ATOMIC);
-	rcu_read_unlock();
+	src = kstrdup(src_node, GFP_KERNEL);
+	dst = kstrdup(dst_node, GFP_KERNEL);
 
 	if (!src || !dst) {
 		ret = -ENOMEM;
@@ -105,7 +152,6 @@ static int icc_get_set(void *data, u64 val)
 	kfree(src);
 	kfree(dst);
 out:
-	mutex_unlock(&debugfs_lock);
 	return ret;
 }
 
@@ -115,7 +161,7 @@ static int icc_commit_set(void *data, u64 val)
 {
 	int ret;
 
-	mutex_lock(&debugfs_lock);
+	guard(mutex)(&debugfs_lock);
 
 	if (!cur_path) {
 		ret = -EINVAL;
@@ -130,7 +176,6 @@ static int icc_commit_set(void *data, u64 val)
 	icc_set_tag(cur_path, tag);
 	ret = icc_set_bw(cur_path, avg_bw, peak_bw);
 out:
-	mutex_unlock(&debugfs_lock);
 	return ret;
 }
 
@@ -160,8 +205,10 @@ int icc_debugfs_client_init(struct dentry *icc_dir)
 
 	client_dir = debugfs_create_dir("test_client", icc_dir);
 
-	debugfs_create_str("src_node", 0600, client_dir, &src_node);
-	debugfs_create_str("dst_node", 0600, client_dir, &dst_node);
+	debugfs_create_file("src_node", 0600, client_dir, &src_node,
+			    &icc_node_fops);
+	debugfs_create_file("dst_node", 0600, client_dir, &dst_node,
+			    &icc_node_fops);
 	debugfs_create_file("get", 0200, client_dir, NULL, &icc_get_fops);
 	debugfs_create_u32("avg_bw", 0600, client_dir, &avg_bw);
 	debugfs_create_u32("peak_bw", 0600, client_dir, &peak_bw);
-- 
2.51.0