linux: fix regressions from PR #2054 performance improvements#2060
linux: fix regressions from PR #2054 performance improvements#2060kolyshkin merged 2 commits intocontainers:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a potential mount leak vulnerability where detached mounts, specifically those created using Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
jnovy
force-pushed
the
fix-mount-propagation-leak
branch
from
7402d14 to
44ed9fa
Compare
kolyshkin
left a comment
kolyshkin
left a comment
There was a problem hiding this comment.
Can we have a test case added?
src/libcrun/container.c
Outdated
| return ret; | ||
|
|
||
| ret = libcrun_container_run_internal (container, context, NULL, false, err); | ||
| ret = libcrun_container_run_internal (container, context, NULL, true, err); |
There was a problem hiding this comment.
It looks like after this change all calls to libcrun_container_run_internal have reset_signal_handlers == true. If that's what you want, then it makes more sense to revert commit 38e1719.
…n path" This reverts commit 38e1719. Skipping signal handler reset in the non-prefork paths caused containers to inherit SIG_IGN for SIGPIPE from the Go runtime parent, breaking "trap ... SIGPIPE" in shells. Fixes: containers#2058 Signed-off-by: Jindrich Novy <jnovy@redhat.com>
Detached mounts from open_tree(OPEN_TREE_CLONE) do not inherit propagation from the parent mount tree, so they keep shared propagation when root_propagation_private skips the MS_PRIVATE call in do_mount(). Set attr.propagation = MS_PRIVATE in get_bind_mount() to prevent mount leaks into the host namespace. Add a regression test that verifies bind mounts inside the container have private propagation. Fixes: containers#2059 Signed-off-by: Jindrich Novy <jnovy@redhat.com>
jnovy
force-pushed
the
fix-mount-propagation-leak
branch
from
44ed9fa to
c4e3fd2
Compare
jnovy
changed the title
|
@kolyshkin Added test and reverted 38e1719 |
|
What do people think of running the entire podman system test suite, maybe just local-root and local-rootless on crun PRs? Need not block this PR. These can be added to the existing testing-farm jobs. |
yes, that would be helpful |
|
@kolyshkin please merge if you are fine with the last version |
4 participants
Two fixes for regressions introduced by PR #2054:
Commit 1: Revert 38e1719 (Fixes #2058)
Skipping signal handler reset in the non-prefork paths caused containers to inherit SIG_IGN for SIGPIPE from the Go runtime parent, breaking
trap ... SIGPIPEin shells.Commit 2: Set MS_PRIVATE on detached mounts (Fixes #2059)
Detached mounts from
open_tree(OPEN_TREE_CLONE)do not inherit propagation from the parent mount tree, so they keep shared propagation whenroot_propagation_privateskips theMS_PRIVATEcall indo_mount(). Setattr.propagation = MS_PRIVATEinget_bind_mount()to prevent mount leaks into the host namespace. Includes a regression test.Fixes: #2058
Fixes: #2059