All of lore.kernel.org
 help / color / mirror / Atom feed
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	[thread overview]
Message-ID: <20210215145305.283064-1-alex_y_xu@yahoo.ca> (raw)
In-Reply-To: 20210215145305.283064-1-alex_y_xu.ref@yahoo.ca

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


       reply	other threads:[~2021-02-15 14:54 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20210215145305.283064-1-alex_y_xu.ref@yahoo.ca>
2021-02-15 14:53 ` Alex Xu (Hello71) [this message]
2021-02-16  0:49   ` [PATCH] proc_sysctl: clamp sizes using table->maxlen Alex Xu (Hello71)
2021-02-16  8:47   ` Christoph Hellwig
2021-02-27 14:41     ` Alex Xu (Hello71)
2021-02-16 12:12   ` [proc_sysctl] 459b3085f2: sysctl_table_check_failed kernel test robot
2021-02-16 12:12     ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210215145305.283064-1-alex_y_xu@yahoo.ca \
    --to=alex_y_xu@yahoo.ca \
    --cc=adobriyan@gmail.com \
    --cc=hch@lst.de \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=rdna@fb.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=yzaikin@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.