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

build-fhs-userenv-bubblewrap: Preserve symlinks in /etc #183874

Merged
merged 2 commits into from
Mar 18, 2023

Conversation

zhaofengli
Copy link
Member

Description of changes

If the original file in /etc is a symlink, make it a symlink inside the sandbox as well. This PR is a more complete solution to #126234 (comment), replacing #145258.

If you bind-mount an individual file and the file is deleted then recreated, the mountpoint will still refer to the original file and cannot be further bind-mounted:

touch mp1 mp2
echo "original content" >file
sudo mount -o bind file mp1
cat mp1 # "original content"

# delete and recreate the file
rm file
echo "new content" >file
cat mp1 # "original content"

# the steam soldier runtime uses bwrap internally, causing another bind-mount
sudo mount -o bind mp1 mp2 # "mount(2) system call failed: No such file or directory."

Since both /etc/static and /run are bind-mounted already, we should just keep the symlinks inside the sandbox.

Tested in the same manner as in #126234 (comment). I can launch games after disconnecting and reconnecting to Wi-Fi with steam -steamdeck.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

Copy link
Contributor

@queezle42 queezle42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a solid fix for #126234 and similar issues.

LGTM

@sg-qwt
Copy link
Contributor

sg-qwt commented Feb 14, 2023

This patch has been sitting for a while. Do we know what's blocking this patch from merged?

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sg-qwt
Copy link
Contributor

sg-qwt commented Mar 10, 2023

I believe so. It'd also solve many other trick issues caused by missing symlink in sandbox (eg, wrong timezone in steam client)

sg-qwt added a commit to sg-qwt/nixos that referenced this pull request Mar 11, 2023
Copy link
Member

@lheckemann lheckemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this, some non-blocking nits though.

@zhaofengli zhaofengli force-pushed the bwrap-fhs-preserve-etc-symlink branch from 98773c9 to f86664b Compare March 17, 2023 17:00
zhaofengli and others added 2 commits March 17, 2023 11:03
If the original file in /etc is a symlink, make it a symlink inside
the sandbox as well.

This fixes NixOS#126234 (comment)

Co-authored-by: Linus Heckemann <git@sphalerite.org>
@zhaofengli zhaofengli force-pushed the bwrap-fhs-preserve-etc-symlink branch from f86664b to 42ef5de Compare March 17, 2023 17:05
@lheckemann lheckemann merged commit 1ba1b35 into NixOS:master Mar 18, 2023
@zhaofengli zhaofengli deleted the bwrap-fhs-preserve-etc-symlink branch March 18, 2023 15:12
SuperSandro2000 added a commit to SuperSandro2000/nixpkgs that referenced this pull request Mar 19, 2023
SuperSandro2000 added a commit to SuperSandro2000/nixpkgs that referenced this pull request Mar 19, 2023
@hellwolf
Copy link
Contributor

hellwolf commented Feb 4, 2024

Back link a related fix in #286360

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants