lkml.org 
[lkml]   [2020]   [Aug]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH 3/6] proc: allocate count + 1 for our read buffer
Date
Al suggested that if we allocate enough space to add in the '\0'
character at the end of our strings, we could just use scnprintf() in
our ->proc_handler functions without having to be fancy about keeping
track of space. There are a lot of these handlers, so the follow ups
will be separate, but start with allocating the extra byte to handle the
null termination of strings.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
fs/proc/proc_sysctl.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 8e19bad83b45..446e7a949025 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -548,6 +548,7 @@ static ssize_t proc_sys_call_handler(struct file *filp, void __user *ubuf,
struct ctl_table *table = PROC_I(inode)->sysctl_entry;
void *kbuf;
ssize_t error;
+ size_t orig_count = count;

if (IS_ERR(head))
return PTR_ERR(head);
@@ -577,9 +578,23 @@ static ssize_t proc_sys_call_handler(struct file *filp, void __user *ubuf,
goto out;
}
} else {
- kbuf = kvzalloc(count, GFP_KERNEL);
+ /*
+ * To make our lives easier in ->proc_handler, we allocate an
+ * extra byte to allow us to use scnprintf() for handling the
+ * buffer output. This works properly because scnprintf() will
+ * only return the number of bytes that it was able to write
+ * out, _NOT_ including the NULL byte. This means the handler's
+ * will only ever return a maximum of count as what they've
+ * copied.
+ *
+ * HOWEVER, we do not assume that ->proc_handlers are without
+ * bugs, so further down we'll do an extra check to make sure
+ * that count isn't larger than the orig_count.
+ */
+ kbuf = kvzalloc(count + 1, GFP_KERNEL);
if (!kbuf)
goto out;
+ count += 1;
}

error = BPF_CGROUP_RUN_PROG_SYSCTL(head, table, write, &kbuf, &count,
@@ -593,6 +608,13 @@ static ssize_t proc_sys_call_handler(struct file *filp, void __user *ubuf,
goto out_free_buf;

if (!write) {
+ /*
+ * This shouldn't happen, but those are the last words before
+ * somebody adds a security vulnerability, so just make sure
+ * that count isn't larger than orig_count.
+ */
+ if (count > orig_count)
+ count = orig_count;
error = -EFAULT;
if (copy_to_user(ubuf, kbuf, count))
goto out_free_buf;
--
2.24.1
\
 
 \ /
  Last update: 2020-08-13 23:04    [W:0.139 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site