Set linux compatible commandline for Chrome. |
|
Issue descriptionChrome currently sets command line by overwriting the given argv. https://cs.chromium.org/chromium/src/services/service_manager/embedder/set_process_title_linux.cc Specifically, the args are concat'ed by white space (0x20). This is incompatible with linux: http://man7.org/linux/man-pages/man5/proc.5.html "The command-line arguments appear in this file as a set of strings separated by null bytes ('\0'), with a further null byte after the last string." This confuses some tools, e.g. gopsutil.Process.CmdlineSlice() family. Nice to be fixed. mdm@, please let us know if there is special requirements that needs to be concat'ed by whitespace instead of null byte. cf) https://bugs.gentoo.org/477538
,
Sep 21
Hey, so, I totally don't work on Chromium anymore, but I can provide a bit of help here. See the comment at the beginning of set_process_title_linux.cc - it explains how the arguments and environment are laid out in memory, and how the kernel treats that memory when it's reading it to expose data via /proc and other such places. In particular, because of the behavior explained in the paragraph on lines 21-27, we must use a single space-separated string, since otherwise it's possible that we'll get unlucky and end up with a 0 byte at just the wrong place and the argument list will be truncated.
,
Sep 21
thanks, that comment block is very helpful to provide background.
i believe that limitation is outdated as of linux-3.5 (July 2012). that's when PR_SET_MM_{ARG,ENV}_{START,END} knobs were introduced to prctl. although it was under the CONFIG_CHECKPOINT_RESTORE knob, and wasn't made generally available until linux-3.10 (June 2013).
http://man7.org/linux/man-pages/man2/prctl.2.html
so if that is the only concern wrt formatting, i think we can handle that:
- create a NUL delimited buffer and point at it using prctl
- if prctl fails with EINVAL, fallback to the whitespace form we have today and rewriting the argv page
,
Sep 24
That sounds like it should work to me.
,
Sep 24
hidehiko@: did you feel like picking this up ? i'm happy to review :).
,
Sep 24
It's a nice idea; however, prctl(PR_SET_MM, PR_SET_MM_ARG_START, ...) requires CAP_SYS_RESOURCE. You get EPERM otherwise. prctl(PR_SET_MM, PR_SET_MM_ARG_START, 0x5594979de260, 0, 0) = -1 EPERM (Operation not permitted)
,
Sep 26
Would it be acceptable to simply not overwrite the command line on Linux? Or only do it when the replacement string fits in the initially allocated memory?
Is the only purpose behind this to get rid of references to /proc/self/exe? If that's the case, we could just replace argv[0] with a short string ("chrome" or "chromium" without clobbering the environ memory.
,
Sep 28
It also has the nice effect of making it so you can see which processes are the renderers, and what arguments are actually passed to them. Otherwise you'd always see "/proc/self/exe --type=zygote" and that's pretty much it. (At least, that'd have been the case years ago, last time I looked at this.)
,
Oct 3
This is causing test failures in tast.ui.MashLogin that look for --mash-service-name in the command line to detect certain process types (similar to looking for --type=renderer, etc.). The code can't use --type because these are actually utility processes. See issue 891470 I think there's another bug on this issue as well.
,
Oct 3
,
Oct 5
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/f95075b921b5041b81dc53cdae911c5bfb83239e commit f95075b921b5041b81dc53cdae911c5bfb83239e Author: James Cook <jamescook@chromium.org> Date: Fri Oct 05 06:00:17 2018 tast-tests: Use arg prefix for mash processes in ui.MashLogin Chrome sometimes truncates arguments in its command line. Instead of looking for "mash-service-name", just look for "mash-". The command line problem is issue 887875 BUG= chromium:891470 ,chromium:887875 TEST=this is the test Change-Id: I812ffd147ec36148cd6185b73510dfef40b70135 Reviewed-on: https://chromium-review.googlesource.com/c/1260054 Tested-by: James Cook <jamescook@chromium.org> Trybot-Ready: James Cook <jamescook@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> Reviewed-by: James Cook <jamescook@chromium.org> [modify] https://crrev.com/f95075b921b5041b81dc53cdae911c5bfb83239e/src/chromiumos/tast/local/bundles/cros/ui/mash_login.go
,
Oct 15
I tried calling prctl(PR_SET_MM, PR_SET_MM_ARG_START, buffer, 0, 0) on-device with buffers in the heap, data area, and stack. All return -1 EPERM, consistent with comment 6 re: CAP_SYS_RESOURCE. I'm guessing we can't or shouldn't grant CAP_SYS_RESOURCE to the browser? |
|
►
Sign in to add a comment |
|
Comment 1 by vapier@chromium.org
, Sep 21Components: Internals>Core
Labels: OS-Chrome OS-Linux