draios / sysdig

Linux system exploration and troubleshooting tool with first class support for containers
http://www.sysdig.com/
Other
7.73k stars 726 forks source link

Invalid executable path in execve argument not shown in trace #73

Open charles-dyfis-net opened 10 years ago

charles-dyfis-net commented 10 years ago

If strace shows the following:

9333  execve("/bin/i-dont-exist", ["/bin/i-dont-exist"], [/* 37 vars */]) = -1 ENOENT (No such file or directory)

...then I get the following for that same call from sysdig:

33504 15:07:27.665180219 0 <NA> (9333) > execve 
33505 15:07:27.665205672 0 <NA> (9333) < execve res=-2(ENOENT) exe= args= tid=9333(<NA>) pid=9333(<NA>) ptid=9319(strace) cwd=/home/cduffy/VC/config-server fdlimit=4096 
33506 15:07:27.665287065 0 <NA> (9333) > switch next=9319(strace) 

Note the exe=, showing an empty string passed for the destination path, whereas strace shows the actual argument given.

gianlucaborello commented 10 years ago

Confirmed. In f_proc_startupdate() (ppm_fillers.c) we use current->mm to get the command line of the new executable, which doesn't work when execve fails. In that case, it should be possible to just parse the execve arguments and copy the array of strings from userspace. There are similar examples in fs/exec.c in the kernel code.

Any volunteer? :)

dkogan commented 9 years ago

I volunteer!!!

So I have a prototype branch that does this. There's quite a bit of code in fs/exec.c that I'd love to reuse, but it's not exported. Current strategy is to copy it to our driver, possibly simplifying where possible. Is this what we want?

gianlucaborello commented 9 years ago

Unfortunately that's the only way we're aware of, which is why code like this:

https://github.com/draios/sysdig/commit/a37b49849ca9fc64de8822e4ff319722daff5757

was never merged into master in the first place. However, parsing the exe from the arguments list should be easier and should not require any locking, making it less bug-prone.

dkogan commented 9 years ago

OK. I have a fix. Due to https://github.com/draios/sysdig/issues/352 I tested this on an older release, not on the dev branch. In the areas the code touches they don't differ, so I imagine it works in dev too. The tested tree is at https://github.com/dkogan/sysdig/tree/failed_execve_print_master and the untested dev-rebased tree is at https://github.com/dkogan/sysdig/tree/failed_execve_print_dev

One aspect I'm unsure about is what to do with args->event_type. I don't know what the difference is between PPME_SYSCALL_EXECVE_8_X and PPME_SYSCALL_EXECVE_13_X and so on. So I report exe,args only for 8,13,14 (dev branch has more but I'm not testing for them) and env only for 14 or 16 (whatever the code that was already there is doing). And I'm not doing any of this for clone()

dkogan commented 9 years ago

So #352 appears to not actually be a problem, so I'm now focusing only on the dev branch. I just deleted my failed_execve_print_master branch, and pushed a small correction to failed_execve_print_dev

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.