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

DRAFT: run: add --host-share-pids to reuse host pid namespace #5441

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chergert
Copy link
Contributor

This allows exposing a process to the PID namespace of the host. That can be useful when you need to expose tooling that will process various /proc information such as profilers and systems tooling.

Typically this is not enabled because once you do, processes can use ptrace on each other and generally circumvent the sandbox.

This is mostly a draft as it really depends on what we'd like to do with #3922

If we decide to go ahead, I can take care of adding documentation, etc.

This allows exposing a process to the PID namespace of the host. That can
be useful when you need to expose tooling that will process various /proc
information such as profilers and systems tooling.

Typically this is not enabled because once you do, processes can use
ptrace on each other and generally circumvent the sandbox.
@orowith2os
Copy link

For this to be actually useful, wouldn't it need to be a static permission that apps opt in to?

Flathub would then, of course, need to block it by default too, with exceptions for those apps that need it.

@chergert
Copy link
Contributor Author

chergert commented Sep 7, 2023

For this to be actually useful, wouldn't it need to be a static permission that apps opt in to?

Flathub would then, of course, need to block it by default too, with exceptions for those apps that need it.

Not at all. Builder could use it when running applications under the profiler so that data can be correlated while recording.

That doesn’t need a public permission at all.

@orowith2os
Copy link

Not at all. Builder could use it when running applications under the profiler so that data can be correlated while recording.

And what for system monitors, like Mission Center? That's one use case where this would be very nice to have. Right now it's constantly spawning a static binary on the host, and it's not fun. It would simplify everything if this was a normal permission apps could just opt into.

@swick
Copy link
Contributor

swick commented Sep 8, 2023

And what for system monitors, like Mission Center?

Out of scope. Not every use case has to be covered by a single feature.

Right now it's constantly spawning a static binary on the host, and it's not fun.

It seems to work, so what's the issue?

@orowith2os
Copy link

Out of scope. Not every use case has to be covered by a single feature.

It just seems like common sense to roll in a permission here, at least to me. This is partially working as-is, and I'd like it to be fully working from the start.

It seems to work, so what's the issue?

One could view this as an app or proxy problem, not a hack problem, but it appears to be a contributor to a memory leak somewhere, and the information it gets is wrong. But I need to check and make sure that problem still exists.

@chergert
Copy link
Contributor Author

chergert commented Sep 8, 2023

Sysprof is in a similar situation, but more extreme. Because not only does it need access to /proc to extract information about mapped files, mount namespaces and the like, but it needs to get kallsyms without redactions, open perf_event streams, etc.

That isn't needed for the sysprof-agent though where we really just need getpid() to return the right thing so that data frames can be correlated with their actual process attributes.

I'll probably be doing some work to address this over the GNOME 46 cycle though and I'd expect it to be solved in a very different manner than this.

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.

None yet

3 participants