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

core/namespace: Protect{Hostname,KernelTunables} and ProcSubset=pid #22206

Closed
brauner opened this issue Jan 21, 2022 · 4 comments · Fixed by #22203
Closed

core/namespace: Protect{Hostname,KernelTunables} and ProcSubset=pid #22206

brauner opened this issue Jan 21, 2022 · 4 comments · Fixed by #22203
Labels

Comments

@brauner
Copy link
Contributor

brauner commented Jan 21, 2022

This is with systemd 250.

Starting any service with:

ProtectHostname=true
ProtectSubset=pid

currently fails. A fix for this is waiting in #22203. That fix should be sound.

What I don't understand and would really like an explanation for is why the addition of ProtectKernelTunables=true makes a service with the aforementioned options work again:

ProtectHostname=true
ProtectSubset=pid
ProtectKernelTunables=true

I can understand it conceptually of course since ProtectSubset=pid implicitly protects the files under ProtectKernelTunables and ProtectHostname by not even exposing them anymore in proc. But I don't understand where in the code and how protect_kernel_tunables and protect_hostname interact with each other such that protect_kernel_tunables cancels protect_hostname and makes things work again.

@brauner brauner changed the title core/namespace: Protect{Hostname,Subset} and ProcSubset=pid core/namespace: Protect{Hostname,KernelTunables} and ProcSubset=pid Jan 21, 2022
@yuwata yuwata added the pid1 label Jan 21, 2022
@bluca
Copy link
Member

bluca commented Jan 21, 2022

/cc @topimiettinen

@topimiettinen
Copy link
Contributor

ProtectKernelTunables=yes' makes a mount of /proc/syswith readonly flags. Maybe it defeatsProcSubset=pidonly` by mounting the directory even if it wouldn't exist then?

brauner added a commit to brauner/systemd that referenced this issue Jan 24, 2022
Rename the normalize_mounts() helper to drop_unused_mounts. All the
helper called in there drop mounts that are unused for a variety of
reasons. But the helper it self speaks of "normalizing" mounts which
sounds like paths are simplified etc. Make it more obvious what it does
by renaming it and by documenting it.

Link: systemd#22206
@brauner
Copy link
Contributor Author

brauner commented Jan 24, 2022

I found the culprit; it's normalize_mounts(). I renamed that function to drop_unused_mounts() and documented it a bit.

brauner added a commit to brauner/systemd that referenced this issue Jan 24, 2022
Rename the normalize_mounts() helper to drop_unused_mounts. All the
helpers called in there get rid of mounts that are unused for a variety
of reasons. And whereas the helpers are aptly prefixed with "drop" the
overall helper isn't and instead uses "normalize".

Make it more obvious what the helper actually does by renaming the it
from normalize_mounts() to drop_unused_mounts(). Readers of code calling
this helper  will immediately see that it will get rid of unused mounts.

Link: systemd#22206
brauner added a commit to brauner/systemd that referenced this issue Jan 24, 2022
Rename the normalize_mounts() helper to drop_unused_mounts. All the
helpers called in there get rid of mounts that are unused for a variety
of reasons. And whereas the helpers are aptly prefixed with "drop" the
overall helper isn't and instead uses "normalize".

Make it more obvious what the helper actually does by renaming it from
normalize_mounts() to drop_unused_mounts(). Readers of code calling this
helper will immediately see that it will get rid of unused mounts.

Link: systemd#22206
@bluca
Copy link
Member

bluca commented Jan 24, 2022

#22203

@bluca bluca closed this as completed Jan 24, 2022
bluca pushed a commit to systemd/systemd-stable that referenced this issue Feb 15, 2022
Rename the normalize_mounts() helper to drop_unused_mounts. All the
helpers called in there get rid of mounts that are unused for a variety
of reasons. And whereas the helpers are aptly prefixed with "drop" the
overall helper isn't and instead uses "normalize".

Make it more obvious what the helper actually does by renaming it from
normalize_mounts() to drop_unused_mounts(). Readers of code calling this
helper will immediately see that it will get rid of unused mounts.

Link: systemd/systemd#22206
(cherry picked from commit fbf90c0)
bluca pushed a commit to systemd/systemd-stable that referenced this issue Feb 15, 2022
Rename the normalize_mounts() helper to drop_unused_mounts. All the
helpers called in there get rid of mounts that are unused for a variety
of reasons. And whereas the helpers are aptly prefixed with "drop" the
overall helper isn't and instead uses "normalize".

Make it more obvious what the helper actually does by renaming it from
normalize_mounts() to drop_unused_mounts(). Readers of code calling this
helper will immediately see that it will get rid of unused mounts.

Link: systemd/systemd#22206
(cherry picked from commit fbf90c0)
(cherry picked from commit 09936a7)
bluca pushed a commit to bluca/systemd-stable that referenced this issue Nov 4, 2022
Rename the normalize_mounts() helper to drop_unused_mounts. All the
helpers called in there get rid of mounts that are unused for a variety
of reasons. And whereas the helpers are aptly prefixed with "drop" the
overall helper isn't and instead uses "normalize".

Make it more obvious what the helper actually does by renaming it from
normalize_mounts() to drop_unused_mounts(). Readers of code calling this
helper will immediately see that it will get rid of unused mounts.

Link: systemd/systemd#22206
(cherry picked from commit fbf90c0)
(cherry picked from commit 09936a7)
(cherry picked from commit 2540b0e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

4 participants