-
Notifications
You must be signed in to change notification settings - Fork 1.5k
usertrap: disable syscall patching when ptraced #12325
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
Conversation
|
IIRC the issue with debuggers and systrap is that they enable single-stepping, which is completely incompatible with the "syshandler" routine used to handle patched syscalls. When the process is inside this routine and subsequently receives a SIGTRAP for every instruction, the signal handler proceeds to blow away the register state of the user process. So this patch is insufficient, because if the workload enters some piece of code where patches have already been applied before patching got disabled, this will result in exactly the same error. I've described what I think the proper solution is in #11649 (we need to roll back the patches in addition to disabling them). All that said I don't see too much harm with merging this (maybe with a better comment and a TODO to implement the rest). This partial fix may be enough for a good chunk of ptraced workloads. @avagin WDYT? |
|
You are right, I guess it will not work when you ptrace-attach to an already existing process that has been patched, but it will work better for programs that are ptraced as soon as cloned, right? I will improve the comment! |
cfc5852 to
62cb91e
Compare
|
I've rewritten the comment and added the TODO. Let me know what else I can do! |
|
Sorry for not replying for a while, I was out. Thanks for fixing the comment! I think you should also add a loud warning log in case |
|
@konstantin-s-bogom interestingly, I hit the the warning even by just running |
Workaround the issue in #12266 . It's not clear to me yet why the issue appears, but it happens intermittently when ptracing a program. I think also #11649 is related. This patch is similar to `--systrap-disable-syscall-patching` but instead of disabling syscall patching globally, we just disable it for some tasks. What do you think? FUTURE_COPYBARA_INTEGRATE_REVIEW=#12325 from trail-of-forks:ptrace-issue 3c52fed PiperOrigin-RevId: 840404296
Because gdb attaches to a child process before it execs the target binary... |
|
Thanks! |
Workaround the issue in #12266 .
It's not clear to me yet why the issue appears, but it happens intermittently when ptracing a program. I think also #11649 is related. This patch is similar to
--systrap-disable-syscall-patchingbut instead of disabling syscall patching globally, we just disable it for some tasks.What do you think?