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 <chenyichong@uniontech.com>
Link: https://patch.msgid.link/20260807100053.1089834-2-chenyichong@uniontech.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Yichong Chen 2026-08-07 18:00:51 +08:00 committed by Greg Kroah-Hartman
parent f1850e443b
commit 493daf5bf2

View File

@ -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;
}
@ -162,8 +207,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);