[PATCH] proc: Add a way to make proc files writable

[PATCH] proc: Add a way to make proc files writable

[Qingjie Xing]

Provide an extra function, proc_create_single_write_data() that
act like its non-write version but also set a write method in
the proc_dir_entry struct. Alse provide a macro
proc_create_single_write to reduces the boilerplate code in the callers.

Signed-off-by: Qingjie Xing <xqjcool@gmail.com>
---
 fs/proc/generic.c       | 18 ++++++++++++++++++
 include/linux/proc_fs.h |  8 +++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index dbe82cf23ee4..0f32a92195fc 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -641,6 +641,7 @@ static const struct proc_ops proc_single_ops = {
 	.proc_read_iter = seq_read_iter,
 	.proc_lseek	= seq_lseek,
 	.proc_release	= single_release,
+	.proc_write	= proc_simple_write,
 };
 
 struct proc_dir_entry *proc_create_single_data(const char *name, umode_t mode,
@@ -658,6 +659,23 @@ struct proc_dir_entry *proc_create_single_data(const char *name, umode_t mode,
 }
 EXPORT_SYMBOL(proc_create_single_data);
 
+struct proc_dir_entry *proc_create_single_write_data(const char *name,
+		umode_t mode, struct proc_dir_entry *parent,
+		int (*show)(struct seq_file *, void *), proc_write_t write,
+		void *data)
+{
+	struct proc_dir_entry *p;
+
+	p = proc_create_reg(name, mode, &parent, data);
+	if (!p)
+		return NULL;
+	p->proc_ops = &proc_single_ops;
+	p->single_show = show;
+	p->write = write;
+	return proc_register(parent, p);
+}
+EXPORT_SYMBOL(proc_create_single_write_data);
+
 void proc_set_size(struct proc_dir_entry *de, loff_t size)
 {
 	de->size = size;
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 0b2a89854440..488d0b76a06f 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -102,7 +102,13 @@ struct proc_dir_entry *proc_create_single_data(const char *name, umode_t mode,
 		int (*show)(struct seq_file *, void *), void *data);
 #define proc_create_single(name, mode, parent, show) \
 	proc_create_single_data(name, mode, parent, show, NULL)
- 
+struct proc_dir_entry *proc_create_single_write_data(const char *name,
+		umode_t mode, struct proc_dir_entry *parent,
+		int (*show)(struct seq_file *, void *), proc_write_t write,
+		void *data);
+#define proc_create_single_write(name, mode, parent, show, write) \
+	proc_create_single_write_data(name, mode, parent, show, write, NULL)
+
 extern struct proc_dir_entry *proc_create_data(const char *, umode_t,
 					       struct proc_dir_entry *,
 					       const struct proc_ops *,
-- 
2.43.0

[PATCH] proc: Add a way to make proc files writable

[Qingjie Xing]

Provide an extra function, proc_create_single_write_data() that
act like its non-write version but also set a write method in
the proc_dir_entry struct. Alse provide a macro
proc_create_single_write to reduces the boilerplate code in the callers.

Signed-off-by: Qingjie Xing <xqjcool@gmail.com>
---
 fs/proc/generic.c       | 18 ++++++++++++++++++
 include/linux/proc_fs.h |  8 +++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index dbe82cf23ee4..0f32a92195fc 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -641,6 +641,7 @@ static const struct proc_ops proc_single_ops = {
 	.proc_read_iter = seq_read_iter,
 	.proc_lseek	= seq_lseek,
 	.proc_release	= single_release,
+	.proc_write	= proc_simple_write,
 };
 
 struct proc_dir_entry *proc_create_single_data(const char *name, umode_t mode,
@@ -658,6 +659,23 @@ struct proc_dir_entry *proc_create_single_data(const char *name, umode_t mode,
 }
 EXPORT_SYMBOL(proc_create_single_data);
 
+struct proc_dir_entry *proc_create_single_write_data(const char *name,
+		umode_t mode, struct proc_dir_entry *parent,
+		int (*show)(struct seq_file *, void *), proc_write_t write,
+		void *data)
+{
+	struct proc_dir_entry *p;
+
+	p = proc_create_reg(name, mode, &parent, data);
+	if (!p)
+		return NULL;
+	p->proc_ops = &proc_single_ops;
+	p->single_show = show;
+	p->write = write;
+	return proc_register(parent, p);
+}
+EXPORT_SYMBOL(proc_create_single_write_data);
+
 void proc_set_size(struct proc_dir_entry *de, loff_t size)
 {
 	de->size = size;
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 0b2a89854440..488d0b76a06f 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -102,7 +102,13 @@ struct proc_dir_entry *proc_create_single_data(const char *name, umode_t mode,
 		int (*show)(struct seq_file *, void *), void *data);
 #define proc_create_single(name, mode, parent, show) \
 	proc_create_single_data(name, mode, parent, show, NULL)
- 
+struct proc_dir_entry *proc_create_single_write_data(const char *name,
+		umode_t mode, struct proc_dir_entry *parent,
+		int (*show)(struct seq_file *, void *), proc_write_t write,
+		void *data);
+#define proc_create_single_write(name, mode, parent, show, write) \
+	proc_create_single_write_data(name, mode, parent, show, write, NULL)
+
 extern struct proc_dir_entry *proc_create_data(const char *, umode_t,
 					       struct proc_dir_entry *,
 					       const struct proc_ops *,
-- 
2.43.0

Re: [PATCH] proc: Add a way to make proc files writable

[Andrew Morton]

On Thu, 31 Oct 2024 18:39:20 -0700 Qingjie Xing <xqjcool@gmail.com> wrote:

> Provide an extra function, proc_create_single_write_data() that
> act like its non-write version but also set a write method in
> the proc_dir_entry struct. Alse provide a macro
> proc_create_single_write to reduces the boilerplate code in the callers.
> 

Please fully describe the reason for making this change.

Also, we are reluctant to add a new interface to Linux unless we add
new users of that interface at the same time.

Thanks.

Re: [PATCH] proc: Add a way to make proc files writable

[Alexey Dobriyan]

On Thu, Oct 31, 2024 at 07:14:53PM -0700, Andrew Morton wrote:
> On Thu, 31 Oct 2024 18:39:20 -0700 Qingjie Xing <xqjcool@gmail.com> wrote:
> 
> > Provide an extra function, proc_create_single_write_data() that
> > act like its non-write version but also set a write method in
> > the proc_dir_entry struct. Alse provide a macro
> > proc_create_single_write to reduces the boilerplate code in the callers.
> > 
> 
> Please fully describe the reason for making this change.
> 
> Also, we are reluctant to add a new interface to Linux unless we add
> new users of that interface at the same time.

Yeah, /proc outside /proc/${pid} and /proc/sys should be pretty dead.

Re: [PATCH] proc: Add a way to make proc files writable

[Qingjie Xing]

Thank you for your feedback.

The motivation for this change is to simplify the creation and management of writable proc files. Many existing parts of the kernel codebase require writable proc files and currently need to set up a proc_ops struct with both read and write methods, which introduces redundancy and boilerplate code.

By providing proc_create_single_write_data() and the associated macro proc_create_single_write, we reduce the need for each caller to explicitly set up write methods, making the code simpler and more maintainable. This can benefit areas where writable proc files are commonly used, as it streamlines the implementation and improves readability.

In the future, we foresee potential use cases where other components of the kernel may need to adopt this approach for their writable proc files, thus justifying the addition of this interface.

for example:
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 34f68ef74b8f..e6fb61505e51 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -513,26 +513,13 @@ static int pgctrl_show(struct seq_file *seq, void *v)
        return 0;
 }

-static ssize_t pgctrl_write(struct file *file, const char __user *buf,
-                           size_t count, loff_t *ppos)
+static int pgctrl_write(struct file *file, char *data, size_t count)
 {
-       char data[128];
        struct pktgen_net *pn = net_generic(current->nsproxy->net_ns, pg_net_id);

        if (!capable(CAP_NET_ADMIN))
                return -EPERM;

-       if (count == 0)
-               return -EINVAL;
-
-       if (count > sizeof(data))
-               count = sizeof(data);
-
-       if (copy_from_user(data, buf, count))
-               return -EFAULT;
-
-       data[count - 1] = 0;    /* Strip trailing '\n' and terminate string */
-
        if (!strcmp(data, "stop"))
                pktgen_stop_all_threads(pn);
        else if (!strcmp(data, "start"))
@@ -542,22 +529,9 @@ static ssize_t pgctrl_write(struct file *file, const char __user *buf,
        else
                return -EINVAL;

-       return count;
-}
-
-static int pgctrl_open(struct inode *inode, struct file *file)
-{
-       return single_open(file, pgctrl_show, pde_data(inode));
+       return 0;
 }

-static const struct proc_ops pktgen_proc_ops = {
-       .proc_open      = pgctrl_open,
-       .proc_read      = seq_read,
-       .proc_lseek     = seq_lseek,
-       .proc_write     = pgctrl_write,
-       .proc_release   = single_release,
-};
-
 static int pktgen_if_show(struct seq_file *seq, void *v)
 {
        const struct pktgen_dev *pkt_dev = seq->private;
@@ -3982,7 +3956,7 @@ static int __net_init pg_net_init(struct net *net)
                pr_warn("cannot create /proc/net/%s\n", PG_PROC_DIR);
                return -ENODEV;
        }
-       pe = proc_create(PGCTRL, 0600, pn->proc_dir, &pktgen_proc_ops);
+       pe = proc_create_single_write(PGCTRL, 0600, pn->proc_dir, pgctrl_show, pgctrl_write);
        if (pe == NULL) {
                pr_err("cannot create %s procfs entry\n", PGCTRL);
                ret = -EINVAL;