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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20241102064132.73443-1-xqjcool@gmail.com>
Date: Fri,  1 Nov 2024 23:41:32 -0700
From: Qingjie Xing <xqjcool@...il.com>
To: adobriyan@...il.com
Cc: akpm@...ux-foundation.org,
	brauner@...nel.org,
	christophe.jaillet@...adoo.fr,
	linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	viro@...iv.linux.org.uk,
	willy@...radead.org,
	xqjcool@...il.com
Subject: Re: [PATCH] proc: Add a way to make proc files writable

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;