Re: [PATCH] proc: Fix uninitialized byte read in get_mm_cmdline()

From: Jakub Jankowski
Date: Fri Jul 12 2019 - 17:26:08 EST


On 2019-07-12, Alexey Izbyshev wrote:

On 7/12/19 8:46 PM, Alexey Dobriyan wrote:
The proper fix to all /proc/*/cmdline problems is to revert

f5b65348fd77839b50e79bc0a5e536832ea52d8d
proc: fix missing final NUL in get_mm_cmdline() rewrite

5ab8271899658042fabc5ae7e6a99066a210bc0e
fs/proc: simplify and clarify get_mm_cmdline() function

Should this be interpreted as an actual suggestion to revert the patches,
fix the conflicts, test and submit them, or is this more like thinking out
loud? In the former case, will it be OK for long term branches?

get_mm_cmdline() does seem easier to read for me before 5ab8271899658042.
But it also has different semantics in corner cases, for example:

- If there is no NUL at arg_end-1, it reads only the first string in
the combined arg/env block, and doesn't terminate it with NUL.

- If there is any problem with access_remote_vm() or copy_to_user(),
it returns -EFAULT even if some data were copied to userspace.

On the other hand, 5ab8271899658042 was merged not too long ago (about a year),
so it's possible that the current semantics isn't heavily relied upon.

I posted this (corner?) case ~3 months ago, unfortunately it wasn't picked up by anyone: https://lkml.org/lkml/2019/4/5/825
You can treat it as another datapoint in this discussion.


Regards,
Jakub

--
Jakub Jankowski|shasta@xxxxxxxxxxx|https://toxcorp.com/