Skip to content

Commit 3d71254

Browse files
committed
/proc/<pid>/cmdline: remove all the special cases
Start off with a clean slate that only reads exactly from arg_start to arg_end, without any oddities. This simplifies the code and in the process removes the case that caused us to potentially leak an uninitialized byte from the temporary kernel buffer. Note that in order to start from scratch with an understandable base, this simplifies things _too_ much, and removes all the legacy logic to handle setproctitle() having changed the argument strings. We'll add back those special cases very differently in the next commit. Link: https://lore.kernel.org/lkml/20190712160913.17727-1-izbyshev@ispras.ru/ Fixes: f5b6534 ("proc: fix missing final NUL in get_mm_cmdline() rewrite") Cc: Alexey Izbyshev <izbyshev@ispras.ru> Cc: Alexey Dobriyan <adobriyan@gmail.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent 0ecfebd commit 3d71254

File tree

1 file changed

+8
-63
lines changed

1 file changed

+8
-63
lines changed

fs/proc/base.c

Lines changed: 8 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ static int proc_root_link(struct dentry *dentry, struct path *path)
212212
static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
213213
size_t count, loff_t *ppos)
214214
{
215-
unsigned long arg_start, arg_end, env_start, env_end;
215+
unsigned long arg_start, arg_end;
216216
unsigned long pos, len;
217217
char *page;
218218

@@ -223,36 +223,18 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
223223
spin_lock(&mm->arg_lock);
224224
arg_start = mm->arg_start;
225225
arg_end = mm->arg_end;
226-
env_start = mm->env_start;
227-
env_end = mm->env_end;
228226
spin_unlock(&mm->arg_lock);
229227

230228
if (arg_start >= arg_end)
231229
return 0;
232230

233-
/*
234-
* We have traditionally allowed the user to re-write
235-
* the argument strings and overflow the end result
236-
* into the environment section. But only do that if
237-
* the environment area is contiguous to the arguments.
238-
*/
239-
if (env_start != arg_end || env_start >= env_end)
240-
env_start = env_end = arg_end;
241-
242-
/* .. and limit it to a maximum of one page of slop */
243-
if (env_end >= arg_end + PAGE_SIZE)
244-
env_end = arg_end + PAGE_SIZE - 1;
245-
246231
/* We're not going to care if "*ppos" has high bits set */
247-
pos = arg_start + *ppos;
248-
249232
/* .. but we do check the result is in the proper range */
250-
if (pos < arg_start || pos >= env_end)
233+
pos = arg_start + *ppos;
234+
if (pos < arg_start || pos >= arg_end)
251235
return 0;
252-
253-
/* .. and we never go past env_end */
254-
if (env_end - pos < count)
255-
count = env_end - pos;
236+
if (count > arg_end - pos)
237+
count = arg_end - pos;
256238

257239
page = (char *)__get_free_page(GFP_KERNEL);
258240
if (!page)
@@ -262,48 +244,11 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
262244
while (count) {
263245
int got;
264246
size_t size = min_t(size_t, PAGE_SIZE, count);
265-
long offset;
266247

267-
/*
268-
* Are we already starting past the official end?
269-
* We always include the last byte that is *supposed*
270-
* to be NUL
271-
*/
272-
offset = (pos >= arg_end) ? pos - arg_end + 1 : 0;
273-
274-
got = access_remote_vm(mm, pos - offset, page, size + offset, FOLL_ANON);
275-
if (got <= offset)
248+
got = access_remote_vm(mm, pos, page, size, FOLL_ANON);
249+
if (got <= 0)
276250
break;
277-
got -= offset;
278-
279-
/* Don't walk past a NUL character once you hit arg_end */
280-
if (pos + got >= arg_end) {
281-
int n = 0;
282-
283-
/*
284-
* If we started before 'arg_end' but ended up
285-
* at or after it, we start the NUL character
286-
* check at arg_end-1 (where we expect the normal
287-
* EOF to be).
288-
*
289-
* NOTE! This is smaller than 'got', because
290-
* pos + got >= arg_end
291-
*/
292-
if (pos < arg_end)
293-
n = arg_end - pos - 1;
294-
295-
/* Cut off at first NUL after 'n' */
296-
got = n + strnlen(page+n, offset+got-n);
297-
if (got < offset)
298-
break;
299-
got -= offset;
300-
301-
/* Include the NUL if it existed */
302-
if (got < size)
303-
got++;
304-
}
305-
306-
got -= copy_to_user(buf, page+offset, got);
251+
got -= copy_to_user(buf, page, got);
307252
if (unlikely(!got)) {
308253
if (!len)
309254
len = -EFAULT;

0 commit comments

Comments
 (0)