Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
To:     linux-kernel@vger.kernel.org, Luis Chamberlain <mcgrof@kernel.org>,
        Kees Cook <keescook@chromium.org>,
        Christoph Hellwig <hch@lst.de>, Andrey Ignatov <rdna@fb.com>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Iurii Zaikin <yzaikin@google.com>,
        Alexey Dobriyan <adobriyan@gmail.com>,
        linux-fsdevel@vger.kernel.org
Cc:     "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
Subject: [PATCH] proc_sysctl: clamp sizes using table->maxlen
Date:   Mon, 15 Feb 2021 09:53:05 -0500
Message-Id: <20210215145305.283064-1-alex_y_xu@yahoo.ca>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
References: <20210215145305.283064-1-alex_y_xu.ref@yahoo.ca>
Content-Length: 7873
Precedence: bulk

This issue was discussed at [0] and following, and the solution was to
clamp the size at KMALLOC_MAX_LEN. However, KMALLOC_MAX_LEN is a maximum
allocation, and may be difficult to allocate in low memory conditions.

Since maxlen is already exposed, we can allocate approximately the right
amount directly, fixing up those drivers which set a bogus maxlen. These
drivers were located based on those which had copy_x_user replaced in
32927393dc1c, on the basis that other drivers either use builtin proc_*
handlers, or do not access the data pointer. The latter is OK because
maxlen only needs to be an upper limit.

[0] https://lore.kernel.org/lkml/1fc7ce08-26a7-59ff-e580-4e6c22554752@oracle.com/

Fixes: 32927393dc1c ("sysctl: pass kernel pointers to ->proc_handler")
Signed-off-by: Alex Xu (Hello71) <alex_y_xu@yahoo.ca>
---
 drivers/parport/procfs.c       | 20 ++++++++++----------
 fs/proc/proc_sysctl.c          | 13 ++++++++-----
 include/linux/sysctl.h         |  2 +-
 net/core/sysctl_net_core.c     |  1 +
 net/decnet/sysctl_net_decnet.c |  4 ++--
 net/sunrpc/xprtrdma/svc_rdma.c | 18 +++++++++---------
 6 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c
index d740eba3c099..a2eeae73f9fa 100644
--- a/drivers/parport/procfs.c
+++ b/drivers/parport/procfs.c
@@ -280,28 +280,28 @@ static const struct parport_sysctl_table parport_sysctl_template = {
 		{
 			.procname	= "base-addr",
 			.data		= NULL,
-			.maxlen		= 0,
+			.maxlen		= 20,
 			.mode		= 0444,
 			.proc_handler	= do_hardware_base_addr
 		},
 		{
 			.procname	= "irq",
 			.data		= NULL,
-			.maxlen		= 0,
+			.maxlen		= 20,
 			.mode		= 0444,
 			.proc_handler	= do_hardware_irq
 		},
 		{
 			.procname	= "dma",
 			.data		= NULL,
-			.maxlen		= 0,
+			.maxlen		= 20,
 			.mode		= 0444,
 			.proc_handler	= do_hardware_dma
 		},
 		{
 			.procname	= "modes",
 			.data		= NULL,
-			.maxlen		= 0,
+			.maxlen		= 40,
 			.mode		= 0444,
 			.proc_handler	= do_hardware_modes
 		},
@@ -310,35 +310,35 @@ static const struct parport_sysctl_table parport_sysctl_template = {
 		{
 			.procname	= "autoprobe",
 			.data		= NULL,
-			.maxlen		= 0,
+			.maxlen		= 256,
 			.mode		= 0444,
 			.proc_handler	= do_autoprobe
 		},
 		{
 			.procname	= "autoprobe0",
 			.data		= NULL,
-			.maxlen		= 0,
+			.maxlen		= 256,
 			.mode		= 0444,
 			.proc_handler	= do_autoprobe
 		},
 		{
 			.procname	= "autoprobe1",
 			.data		= NULL,
-			.maxlen		= 0,
+			.maxlen		= 256,
 			.mode		= 0444,
 			.proc_handler	= do_autoprobe
 		},
 		{
 			.procname	= "autoprobe2",
 			.data		= NULL,
-			.maxlen		= 0,
+			.maxlen		= 256,
 			.mode		= 0444,
 			.proc_handler	= do_autoprobe
 		},
 		{
 			.procname	= "autoprobe3",
 			.data		= NULL,
-			.maxlen		= 0,
+			.maxlen		= 256,
 			.mode		= 0444,
 			.proc_handler	= do_autoprobe
 		},
@@ -349,7 +349,7 @@ static const struct parport_sysctl_table parport_sysctl_template = {
 		{
 			.procname	= "active",
 			.data		= NULL,
-			.maxlen		= 0,
+			.maxlen		= 256,
 			.mode		= 0444,
 			.proc_handler	= do_active_device
 		},
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index d2018f70d1fa..4a54d3cc174b 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -547,7 +547,7 @@ static ssize_t proc_sys_call_handler(struct kiocb *iocb, struct iov_iter *iter,
 	struct inode *inode = file_inode(iocb->ki_filp);
 	struct ctl_table_header *head = grab_header(inode);
 	struct ctl_table *table = PROC_I(inode)->sysctl_entry;
-	size_t count = iov_iter_count(iter);
+	size_t count = min(table->maxlen, iov_iter_count(iter));
 	char *kbuf;
 	ssize_t error;
 
@@ -567,10 +567,6 @@ static ssize_t proc_sys_call_handler(struct kiocb *iocb, struct iov_iter *iter,
 	if (!table->proc_handler)
 		goto out;
 
-	/* don't even try if the size is too large */
-	error = -ENOMEM;
-	if (count >= KMALLOC_MAX_SIZE)
-		goto out;
 	kbuf = kzalloc(count + 1, GFP_KERNEL);
 	if (!kbuf)
 		goto out;
@@ -1138,6 +1134,13 @@ static int sysctl_check_table(const char *path, struct ctl_table *table)
 		if ((table->mode & (S_IRUGO|S_IWUGO)) != table->mode)
 			err |= sysctl_err(path, table, "bogus .mode 0%o",
 				table->mode);
+
+		if (table->maxlen >= KMALLOC_MAX_SIZE)
+			err |= sysctl_err(path, table, "maxlen %ld too big",
+				table->maxlen);
+
+		if ((table->mode & S_IRUGO) && !table->maxlen)
+			err |= sysctl_err(path, table, "cannot read maxlen=0");
 	}
 	return err;
 }
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 51298a4f4623..78a1d36767f9 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -112,7 +112,7 @@ static inline void *proc_sys_poll_event(struct ctl_table_poll *poll)
 struct ctl_table {
 	const char *procname;		/* Text ID for /proc/sys, or zero */
 	void *data;
-	int maxlen;
+	size_t maxlen;
 	umode_t mode;
 	struct ctl_table *child;	/* Deprecated */
 	proc_handler *proc_handler;	/* Callback for text formatting */
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index d86d8d11cfe4..c51a2e7e0dfb 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -470,6 +470,7 @@ static struct ctl_table net_core_table[] = {
 #ifdef CONFIG_NET_FLOW_LIMIT
 	{
 		.procname	= "flow_limit_cpu_bitmap",
+		.maxlen         = 128,
 		.mode		= 0644,
 		.proc_handler	= flow_limit_cpu_sysctl
 	},
diff --git a/net/decnet/sysctl_net_decnet.c b/net/decnet/sysctl_net_decnet.c
index 67b5ab2657b7..2ca2ac42c40c 100644
--- a/net/decnet/sysctl_net_decnet.c
+++ b/net/decnet/sysctl_net_decnet.c
@@ -239,14 +239,14 @@ static int dn_def_dev_handler(struct ctl_table *table, int write,
 static struct ctl_table dn_table[] = {
 	{
 		.procname = "node_address",
-		.maxlen = 7,
+		.maxlen = DN_ASCBUF_LEN,
 		.mode = 0644,
 		.proc_handler = dn_node_address_handler,
 	},
 	{
 		.procname = "node_name",
 		.data = node_name,
-		.maxlen = 7,
+		.maxlen = sizeof(node_name),
 		.mode = 0644,
 		.proc_handler = proc_dostring,
 	},
diff --git a/net/sunrpc/xprtrdma/svc_rdma.c b/net/sunrpc/xprtrdma/svc_rdma.c
index 526da5d4710b..f326ba6825f2 100644
--- a/net/sunrpc/xprtrdma/svc_rdma.c
+++ b/net/sunrpc/xprtrdma/svc_rdma.c
@@ -143,63 +143,63 @@ static struct ctl_table svcrdma_parm_table[] = {
 	{
 		.procname	= "rdma_stat_read",
 		.data		= &rdma_stat_read,
-		.maxlen		= sizeof(atomic_t),
+		.maxlen		= 32,
 		.mode		= 0644,
 		.proc_handler	= read_reset_stat,
 	},
 	{
 		.procname	= "rdma_stat_recv",
 		.data		= &rdma_stat_recv,
-		.maxlen		= sizeof(atomic_t),
+		.maxlen		= 32,
 		.mode		= 0644,
 		.proc_handler	= read_reset_stat,
 	},
 	{
 		.procname	= "rdma_stat_write",
 		.data		= &rdma_stat_write,
-		.maxlen		= sizeof(atomic_t),
+		.maxlen		= 32,
 		.mode		= 0644,
 		.proc_handler	= read_reset_stat,
 	},
 	{
 		.procname	= "rdma_stat_sq_starve",
 		.data		= &rdma_stat_sq_starve,
-		.maxlen		= sizeof(atomic_t),
+		.maxlen		= 32,
 		.mode		= 0644,
 		.proc_handler	= read_reset_stat,
 	},
 	{
 		.procname	= "rdma_stat_rq_starve",
 		.data		= &rdma_stat_rq_starve,
-		.maxlen		= sizeof(atomic_t),
+		.maxlen		= 32,
 		.mode		= 0644,
 		.proc_handler	= read_reset_stat,
 	},
 	{
 		.procname	= "rdma_stat_rq_poll",
 		.data		= &rdma_stat_rq_poll,
-		.maxlen		= sizeof(atomic_t),
+		.maxlen		= 32,
 		.mode		= 0644,
 		.proc_handler	= read_reset_stat,
 	},
 	{
 		.procname	= "rdma_stat_rq_prod",
 		.data		= &rdma_stat_rq_prod,
-		.maxlen		= sizeof(atomic_t),
+		.maxlen		= 32,
 		.mode		= 0644,
 		.proc_handler	= read_reset_stat,
 	},
 	{
 		.procname	= "rdma_stat_sq_poll",
 		.data		= &rdma_stat_sq_poll,
-		.maxlen		= sizeof(atomic_t),
+		.maxlen		= 32,
 		.mode		= 0644,
 		.proc_handler	= read_reset_stat,
 	},
 	{
 		.procname	= "rdma_stat_sq_prod",
 		.data		= &rdma_stat_sq_prod,
-		.maxlen		= sizeof(atomic_t),
+		.maxlen		= 32,
 		.mode		= 0644,
 		.proc_handler	= read_reset_stat,
 	},
-- 
2.30.1