Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
charles-dyfis-net opened this issue Apr 6, 2014 · 6 comments
Open

Comments

@charles-dyfis-net
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

dkogan commented Apr 16, 2015

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
Copy link
Contributor

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

a37b498

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
Copy link
Contributor

dkogan commented Apr 17, 2015

OK. I have a fix. Due to #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
Copy link
Contributor

dkogan commented Apr 17, 2015

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
Copy link

github-actions bot commented Mar 3, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants