On Wed, May 16, 2018 at 6:21 PM Tejun Heo <tj@xxxxxxxxxx> 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,
