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

common: use ptrace if yama blocked process_vm_readv/writev #280

Merged
merged 1 commit into from
Nov 30, 2016

Conversation

cuviper
Copy link
Contributor

@cuviper cuviper commented Nov 30, 2016

Having sysctl kernel.yama.ptrace_scope=1, one may only call ptrace
attach on direct descendants. The same restriction is also checked for
process_vm_readv/writev and certain procfs files. However, if an
intermediate parent process already exited, we could end up with a
grandchild that we're still ptracing but isn't our descendant, so we
can't use the process_vm functions anymore -> EPERM.

We already had a fallback here for EFAULT, to just use ptrace memory
access, so use the same fallback after EPERM too.

Fixes #274.

Having sysctl kernel.yama.ptrace_scope=1, one may only call ptrace
attach on direct descendants.  The same restriction is also checked for
`process_vm_readv`/`writev` and certain procfs files.  However, if an
intermediate parent process already exited, we could end up with a
grandchild that we're still ptracing but isn't our descendant, so we
can't use the `process_vm` functions anymore -> `EPERM`.

We already had a fallback here for `EFAULT`, to just use `ptrace` memory
access, so use the same fallback after `EPERM` too.

Fixes dyninst#274.
@cuviper
Copy link
Contributor Author

cuviper commented Nov 30, 2016

Did Jenkins actually test this? I found testsuite-build #97, but it doesn't look like it ran anything.

Anyway, I'm not worried about it -- I think this change has very low risk.

@wrwilliams
Copy link
Member

It did, but it failed to aggregate; the results look clean.

May as well merge the proccontrol fixes in whatever order is best, Josh; they should help clean things up for the other testing we're doing on parsing fixes etc.

@cuviper
Copy link
Contributor Author

cuviper commented Nov 30, 2016

Ok, thanks! They're all touching distinct lines, so I'll just merge them in submission order.

@cuviper cuviper merged commit d8807bc into dyninst:master Nov 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants