lkml.org 
[lkml]   [2018]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: [PATCH] proc: Don't allow empty /proc/PID/cmdline for user tasks
On Wed, May 16, 2018 at 6:21 PM Tejun Heo <tj@kernel.org> wrote:

> This patch fixes the issue by making proc_pid_cmdline_read() never
> return empty string for user tasks.

Ugh.

That function really is too damn ugly, and this just makes it worse.

Can we please split things up a bit before uglifying the code further?

IOW, the first step should be something like the attached patch, which
splits up all the tsk/mm error cases.

Then your patch could just be a trivial

if (!*pos && !ret)
ret = get_comm_cmdline(tsk, buf, count, pos);

in that new (and much simpler) proc_pid_cmdline_read() just before the
put_task_struct.

Hmm?

NOTE! This patch is *entirely* untested, but it builds and the conversion
was pretty much entirely mechanical.

And yes, the "get_mm_cmdline()" function is still too damn ugly, and should
still be cleaned up more, but it's at least _slightly_ simpler than it used
to be, and the new logic wouldn't go into that horrible thing.

Linus
fs/proc/base.c | 64 +++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 39 insertions(+), 25 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 1a76d751cf3c..c4d963a12162 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -205,11 +205,9 @@ static int proc_root_link(struct dentry *dentry, struct path *path)
return result;
}

-static ssize_t proc_pid_cmdline_read(struct file *file, char __user *buf,
- size_t _count, loff_t *pos)
+static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
+ size_t _count, loff_t *pos)
{
- struct task_struct *tsk;
- struct mm_struct *mm;
char *page;
unsigned long count = _count;
unsigned long arg_start, arg_end, env_start, env_end;
@@ -218,26 +216,13 @@ static ssize_t proc_pid_cmdline_read(struct file *file, char __user *buf,
char c;
ssize_t rv;

- BUG_ON(*pos < 0);
-
- tsk = get_proc_task(file_inode(file));
- if (!tsk)
- return -ESRCH;
- mm = get_task_mm(tsk);
- put_task_struct(tsk);
- if (!mm)
- return 0;
/* Check if process spawned far enough to have cmdline. */
- if (!mm->env_end) {
- rv = 0;
- goto out_mmput;
- }
+ if (!mm->env_end)
+ return 0;

page = (char *)__get_free_page(GFP_KERNEL);
- if (!page) {
- rv = -ENOMEM;
- goto out_mmput;
- }
+ if (!page)
+ return -ENOMEM;

down_read(&mm->mmap_sem);
arg_start = mm->arg_start;
@@ -365,13 +350,42 @@ static ssize_t proc_pid_cmdline_read(struct file *file, char __user *buf,

out_free_page:
free_page((unsigned long)page);
-out_mmput:
- mmput(mm);
- if (rv > 0)
- *pos += rv;
return rv;
}

+static ssize_t get_task_cmdline(struct task_struct *tsk, char __user *buf,
+ size_t count, loff_t *pos)
+{
+ struct mm_struct *mm;
+ ssize_t ret;
+
+ mm = get_task_mm(tsk);
+ if (!mm)
+ return 0;
+
+ ret = get_mm_cmdline(mm, buf, count, pos);
+ mmput(mm);
+ return ret;
+}
+
+static ssize_t proc_pid_cmdline_read(struct file *file, char __user *buf,
+ size_t count, loff_t *pos)
+{
+ struct task_struct *tsk;
+ ssize_t ret;
+
+ BUG_ON(*pos < 0);
+
+ tsk = get_proc_task(file_inode(file));
+ if (!tsk)
+ return -ESRCH;
+ ret = get_task_cmdline(tsk, buf, count, pos);
+ put_task_struct(tsk);
+ if (ret > 0)
+ *pos += ret;
+ return ret;
+}
+
static const struct file_operations proc_pid_cmdline_ops = {
.read = proc_pid_cmdline_read,
.llseek = generic_file_llseek,
\
 
 \ /
  Last update: 2018-05-17 20:24    [W:0.033 / U:3.516 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site