Messages in this thread | | | Subject | Re: [PATCH] sysctl: Limit the size of I/Os to PAGE_SIZE | From | Josef Bacik <> | Date | Thu, 13 May 2021 12:15:34 -0400 |
| |
On 5/13/21 12:12 PM, Josef Bacik wrote: > On 5/13/21 12:06 PM, Matthew Wilcox (Oracle) wrote: >> We currently allow a read or a write that is up to KMALLOC_MAX_SIZE. >> This has caused problems when cat decides to do a 64kB read and >> so we allocate a 64kB buffer for the sysctl handler to store into. >> The immediate problem was fixed by switching to kvmalloc(), but it's >> ridiculous to allocate so much memory to read what is likely to be a >> few bytes. >> >> sysfs limits reads and writes to PAGE_SIZE, and I feel we should do the >> same for sysctl. The largest sysctl anyone's been able to come up with >> is 433 bytes for /proc/sys/dev/cdrom/info >> >> This will allow simplifying the BPF sysctl code later, but I'll leave >> that for someone who understands it better. >> >> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> >> --- >> fs/proc/proc_sysctl.c | 15 +++++++++------ >> 1 file changed, 9 insertions(+), 6 deletions(-) >> >> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c >> index dea0f5ee540c..a97a8a4ff270 100644 >> --- a/fs/proc/proc_sysctl.c >> +++ b/fs/proc/proc_sysctl.c >> @@ -562,11 +562,14 @@ 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 */ >> + /* reads may return short values; large writes must fail now */ >> + if (count >= PAGE_SIZE) { >> + if (write) >> + goto out; >> + count = PAGE_SIZE; >> + } >> error = -ENOMEM; >> - if (count >= KMALLOC_MAX_SIZE) >> - goto out; >> - kbuf = kvzalloc(count + 1, GFP_KERNEL); >> + kbuf = kmalloc(PAGE_SIZE, GFP_KERNEL); >> if (!kbuf) >> goto out; > > Below here we have > > kbuf[count] = '\0'; >
Nevermind I just re-read it and it's
if (write) kbuf[count] = '\0';
I'm stupid, you can add
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Thanks,
Josef
| |