Skip to content

Fix usage of process_vm_readv in createdump - #50477

Merged
2 commits merged into
dotnet:mainfrom
hoyosjs:juhoyosa/fix-createdump-container
Mar 31, 2021
Merged

Fix usage of process_vm_readv in createdump#50477
2 commits merged into
dotnet:mainfrom
hoyosjs:juhoyosa/fix-createdump-container

Conversation

@hoyosjs

@hoyosjs hoyosjs commented Mar 31, 2021

Copy link
Copy Markdown
Member

This PR tries to use process_vm_readv and falls back to using pread on /proc/mem if the first fails with EPERM. This is needed as docker 19.03 on kernels newer than 4.8 allows to use ptrace as long as yama settings don't restrict it, but it does not allow usage of process_vm_* syscalls. These were added very recently to the allow-list but we need a mechanism to allow customers to collect dumps in their containerized environments.

Fixes dotnet/diagnostics#2098 in main.

@hoyosjs hoyosjs added this to the 6.0.0 milestone Mar 31, 2021
@hoyosjs
hoyosjs requested a review from mikem8361 March 31, 2021 11:29
@hoyosjs hoyosjs self-assigned this Mar 31, 2021
@ghost

ghost commented Mar 31, 2021

Copy link
Copy Markdown

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR tries to use process_vm_readv and falls back to using pread on /proc/mem if the first fails with EPERM. This is needed as docker 19.03 on kernels newer than 4.8 allows to use ptrace as long as yama settings don't restrict it, but it does not allow usage of process_vm_* syscalls. These were added very recently to the allow-list but we need a mechanism to allow customers to collect dumps in their containerized environments.

Fixes dotnet/diagnostics#2098 in main.

Author: hoyosjs
Assignees: hoyosjs
Labels:

area-Diagnostics-coreclr

Milestone: 6.0.0

@hoyosjs
hoyosjs requested a review from sdmaclea March 31, 2021 11:35
Comment thread src/coreclr/debug/createdump/crashinfounix.cpp Outdated
@tommcdon

Copy link
Copy Markdown
Member

fyi @shirhatti

@ghost

ghost commented Mar 31, 2021

Copy link
Copy Markdown

Hello @hoyosjs!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost
ghost merged commit d5aeac9 into dotnet:main Mar 31, 2021
@saul

saul commented Mar 31, 2021

Copy link
Copy Markdown

Thanks for the fix. Can we expect this to be backported to 5.0?

@hoyosjs
hoyosjs deleted the juhoyosa/fix-createdump-container branch March 31, 2021 20:40
@hoyosjs

hoyosjs commented Mar 31, 2021

Copy link
Copy Markdown
Member Author

@saul I've opened the backport PR. Will give the change some bake time against the tests and ask ship room for permission to add to servicing in 5.0.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 30, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dotnet-dump 0x80004005 on .NET 5 in Linux Docker container

4 participants