Process.cmdline() wrong results, parameters not correctly splitted #1179
Comments
bitranox
changed the title
|
The cmdline is calculated by reading |
|
ok, the result of the line above from commandline was : proc.cmdline() result was : (please also note the blank after .... _server.py ') |
|
I finally decided to work around, since I found no solution. maybe its worth looking into it ... |
|
Also reproduced on my side and related to the Glances issue (nicolargo/glances#1192): 4430 is not an Atom/Electron process ==> cmdline() return a list of strings |
|
So, this is the problem. From The reason why you guys see a single string is because for some reason those processes use space ( With that said, I'm not sure what psutil should do in this case. Maybe it can split by space if the cmdline string has no |
|
https://stackoverflow.com/a/1586001 |
import os
pids = [x for x in os.listdir('/proc') if x.isdigit()]
for pid in pids:
try:
with open("/proc/%s/cmdline" % pid) as f:
data = f.read()
except EnvironmentError:
pass
else:
if data and '\x00' not in data:
print(repr(data))
print("")This shows the processes "misbehaving". I have different chrome processes like this and a bunch of others: |
|
OK this is fixed. |
* 'pslisten' of github.com:nlevitt/psutil: (922 commits) Update INSTALL.rst Pass python_requires argument to setuptools (giampaolo#1208) giampaolo#1152: fix doc to mention CLI command necessary to enable disk_io_counters() on win pre release pre release pre release pre-release fix giampaolo#1201: document that timeout kwarg is expressed in seconds Add mount points to disk_partitions() in Windows (giampaolo#775) (giampaolo#1192) add test for cpu_affinity what a stupid bug! (giampaolo#1190) update doc pre release pre-release; also get rid of PSUTIL_DEBUG doc instructions (it's kinda useless for the user after all) Use FutureWarning instead of DeprecationWarning (giampaolo#1188) fix test refactor environ() test Fix OSX pid 0 bug (giampaolo#1187) change assert in test refactor Process.__repr__ Faster Process.children(recursive=True) (giampaolo#1186) Speedup Process.children() (giampaolo#1185) update doc update HISTORY fix giampaolo#1179 / linux / cmdline: handle processes erroneously overwriting /proc/pid/cmdline by using spaces instead of null bytes as args separator set x bit to test_aix.py fix giampaolo#1181: raise AD if task_for_pid() fails with 5 and errno == ENOENT fix posix failure Arguments for NoSuchProcess and AccessDenied for the C ext (giampaolo#1180) fix travis failure https://travis-ci.org/giampaolo/psutil/jobs/306424509 be smarter in searching python exe do not test platf specific modules on wheelhouse try to fix travis failure fix travis failures try to use PYTHON_EXE instead of sys.executable giampaolo#1177: give credits to @wiggin15 OSX: implement sensors_battery (giampaolo#1177) improve error msg for old windows systems giampaolo#811 add debug messages do not mention apt-get as method of installation as it's not recommended syntax highlight in doc files syntax highlight in doc files fix doc indentation 1173 debug mode (giampaolo#1176) code style update MANIFEST giampaolo#1174: use TimeoutExpired in wait_pid() sort imports by name Move exceptions to separate file (giampaolo#1174) appveyor: enable python warnings when running tests refactor winmake.py use a C global variable to figure out whether we're in testing mode fix unicode err define a setup() function which is called on import by all C modules move PyUnicode compt fun definition up in the file rename C func re-enable test on appveyor; remove unused C code refactor PSUTIL_TESTING C APIs inspect PSUTIL_TESTING env var from C again giampaolo#1152: (DeviceIOControl), skip disk on ERROR_INVALID_FUNCTION and ERROR_NOT_SUPPORTED giampaolo#1152 / win / disk_io_counters(): DeviceIOControl errors were ignored; che return value and retry call on ERROR_INSUFFICIENT_BUFFER upgrade dist cmds change make cmds disable IPv6 tests if IPv6 is not supported travis / OSX: run py 3.6 instead of 3.4 fix giampaolo#1169: (Linux) users() hostname returns username instead update README, bump up version get rid of PSUTIL_TESTING env var: it must be necessarily set from cmdline, hence 'python -m psutil.tests' won't work out of the box try to set PSUTIL_TESTING env var from python before failing skip cpu_freq tests if not available (giampaolo#1170) update doc pre-release giampaolo#1053: drop python 3.3 support try to fix appveyor failure; also refactor generate_manifest.py giampaolo#1167 give CREDITS to @matray Including non-unicast packets in packet count calculation (giampaolo#1167) fix giampaolo#1166 (doc mistake) provide a 'make help' command ifconfig.py humanize bytes try to limit false positives on appveyor/windows reap_children() in a finally block in order to limit false positives unicode tests: use different name for test dir fix failure on osx/travis update Makefile fix test giampaolo#1164 give CREDITS to @wiggin15 AIX: implement num_ctx_switches (giampaolo#1164) use new PYTHON_EXE improve logic to determine python exe location add DEVNOTES file; move TODO.aix into _psutil_aix.c Fix test_emulate_energy_full_not_avail (giampaolo#1163) update README try to limit occasional appveyor failure Remove trove classifiers for untested and unsupported platforms (giampaolo#1162) Fix giampaolo#1154: remove 'threads' method on older AIX (giampaolo#1156) give CREDITS to @adpag for giampaolo#1159, giampaolo#1160 and giampaolo#1161 Fix test asserts due to leftover test subprocesses (giampaolo#1161) Fix network tests for newer ifconfig versions. (giampaolo#1160) Fix pre-commit hook for python 3.x. (giampaolo#1159) revert last commit ...
|
This doesn't seem to be completely fixed. In some circumstances the cmdline file can end with a null byte but still be separated by spaces. In this case cmdline() does not return the parameters correctly split. Example: |
|
psutil already bends itself to accomodate for processes manually re-setting the cmdline without following the rules. This looks like yet another corner case. We can't cover all of them. =) |
|
I get where you're coming from, but how else is a program relying on cmdline being accurate going to know if it has been modified? Edit: |
|
Jonny, You can try following: |
|
@bitranox No, unfortunately this does not solve the problem because an argument could have spaces in it, but once you join the arguments together, there are no quotes. E.g. The result of running this command would be that zathura would try to open 3 (non-existent) files instead of 1 real file. |
|
@JonnyHaystack Robert |
|
@bitranox But then that does nothing. It would only work if the arguments were correctly in an array to begin with. If you had |
|
I dont get Your meaning. everything good ? |
|
But you can't cover both cases... If you write something that works for a "bad" cmdline like: It will not work for a good cmdline like: |
|
You might check how /proc/.../cmdline looks like ... maybe the quotes are there ? on my system, /proc/.../cmdline parts are seperated by "0" : |
|
I'm not sure what you're trying to say. If the program modifies its own cmdline, generally it will include quotes where necessary. But you there is no way of telling if the program has done this, and there is no way to reliably interpret the cmdline without knowing if it has been modified into one string. |
|
@JonnyHaystack , I made a new version for You - please try : |
|
@giampaolo |
|
psutil does split it by the null byte, But anyway my problem is that a lot of the processes that modify their own cmdline still end with the null byte, despite not being separated by null bytes. I can't think of a reliable way to distinguish between a legitimate cmdline that just has one argument and a modified cmdline that has multiple arguments stuffed into |
Do You have an example for me ? Did You really check the root process or just some subprocess of the program ? |
|
These are processes obtained from window IDs using xprop. See JonnyHaystack/i3-resurrect#35 (comment) for a real life example. |
|
@JonnyHaystack I investigated a bit, and found some of my own scripts having a commandline like that - if they are started via a systemd startscript. (I defenitely did not change I adapted my script, it should now give back the correct values in both cases, under Windows and Linux. BTW, the lexer I use (with regexp) is about 10x faster than the the original python shlex, if it matters to You at all. Please test it !
|
|
|
ah, yes, sorry, forgot to put it in use: Robert |
|
@JonnyHaystack |
|
I have thought of doing what you are doing, but it doesn't cover the case where the path to the executable contains spaces. See the problem: In the last case, the path to the executable is This is why I say I can't think of a reliable way to distinguish between a valid or modified cmdline. |
|
@JonnyHaystack Before "resurrecting" we need also to change the current directory, because also parameters might contain filenames which are relative to the current active directory will work on it and let You know ... Robert |
|
Oh yeah good idea, I didn't think of just checking if it is an existing executable. That ought to cover all cases nicely! I'm assuming either absolute paths or something in the user's PATH for now at least, so it should be fine with just a simple check. I do already set the cwd when restoring processes. You also have to make sure to set the PWD environment variable if you want the working directory to be set properly. EDIT: |
|
Okay I got it working to the point where it will only fail if cmdline has one argument, which is a relative path to the executable and contains spaces. That's plenty good enough for me. The way I do it is like so: |
|
I've found out that there are some cases where the cmdline ends with multiple null characters at the end, but psutil assumes there can only be one, so it just removes the last character before splitting. The result of this is that you have a load of empty string arguments that shouldn't be there. I also don't think this is the correct thing to do if the cmdline has all been crammed into one string. Why not use shlex.split() in that case? If you split by the space character and there are arguments with spaces, it would break the cmdline to the point where it's impossible to reconstruct and one would be forced to not use psutil in that case. |
|
@JonnyHaystack |
That is expected and there is also a test case for it: psutil/psutil/tests/test_process.py Lines 772 to 774 in 0c5e704 As for this issue: cmdline = proc.cmdline()
if len(cmdline) == 1 and ' ' in cmdline[0]:
cmdline = cmdline[0].split(' ')How many processes like this do you have? 1? Do you know anything about it (app name or parent name)? It would be good to get a sense of how common this is in practice, and whether it is worth to include a fix in psutil or not. To b clear, what I imagine this misbehaving app is doing is:
|
|
Hello Paolo, I have a python program myself, that shows 0x20 instead of 0x00 as a separator between the command and the parameters - first I thought it is because started with systemd daemon, but that was not the case. this was started with systemd service with root rights. If I start it from the console with "sudo" I see following processes : pid 7948 is the process I started If You look at the processlist of Your linux box, You will find for sure some of that misbehaving processes (for instance google chrome subprocesses, they are all separated with 0x20) I think there can not be a 100% working solution, especially if You have parameters with blank in it - because the shell throws away the quoting, and it ends up in a cmdline separated with blanks - so no way to make it 100% correctly working in every case. Anyway, I think You should patch psutil.Process().cmdline a bit - lets assume the original command was : well - this works in MOST of the cases - except if there is a blank in one of the parameters it would be really good to find out the root cause, but I did not succeed - maybe You are more lucky. EDIT1: interesting - on some cases it can be really setproctitle - but not at rpyc, that does not use python setproctitle at all. |
…ends with null byte but args are separated by spaces
Hey there. No, I am still Chinese for a little while. =) |
|
Paolo, Dont know if You considered that we can only find out which part is the execuable (absolute or relative path) or symlink, by actually looking if the file exists. all the rest are parameters for the executable. BTW - this happens only on Linux, not in Windows - there I use just the lexer. Well - that is the best we can do, it still will fail sometimes, but thats the best we can come up with. zhu ni tian tian kuai le |
psutil version 5.4.1
python 3.6.1
Linux Kernel 4.13.0.16
inside an LXC container (if that matters)
for instance:
so I needed to concentate and shlex to split correctly.
Randomly wrong results, maybe because some of those processes were started via systemd service ?
No idea ...
yours sincerely
Robert
The text was updated successfully, but these errors were encountered: