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

xopenat(): introduce new XO_REGULAR flag #35834

Merged
merged 5 commits into from
Jan 7, 2025

Conversation

poettering
Copy link
Member

This is something I think we should have added a long time ago: a flavour of open() that safely ensures the inode we are opening is a regular file, before we open it. It does this by means of pinning the inode via O_PATH first, and after verification actually opening it.

This ports some code over to this, but sooner or later we should probably use this a lot more, so that we don't accidentally open weird stuff such as device nodes or pipes, where we should not.

src/basic/fs-util.c Outdated Show resolved Hide resolved
@github-actions github-actions bot added tests please-review PR is ready for (re-)review by a maintainer labels Jan 3, 2025
@poettering poettering added the ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR label Jan 3, 2025
src/basic/fs-util.c Outdated Show resolved Hide resolved
@yuwata yuwata added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review PR is ready for (re-)review by a maintainer labels Jan 4, 2025
If this flag is set we guarantee that the fd returned refers to a
regular file. If the file exists and is not one, fails.
@github-actions github-actions bot added machine please-review PR is ready for (re-)review by a maintainer and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR labels Jan 6, 2025
…o container

When copying files from or to a container we so far opened the host side
fd first, then entered the container (specifically, joined it's mount
namespace) in a forked off child process, and opened the other side
there, followed by the (potentially slow) copying from inside the
container mount namespace.

This commit changes this so that we rejoin the host mount namespace
before doing the copying routine. This is relevant, so that we can rely
on /proc/self/fd/… to work, which is not the case otherwise, as we'll
see /proc/ from a pidns that is not our own, in wich case
/proc/self/fd/… is refused. By moving back to the host mount namespace
our own pidns and the pidns the /proc/ mount belongs to will be in sync
again, and all is good.

This is in particular preparation for the next commit, that makes the
copy routine strictly depending on /proc/ being accessible and working.
Copy link
Member

@yuwata yuwata left a comment

Choose a reason for hiding this comment

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

LGTM. Interesting.

@yuwata yuwata added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed please-review PR is ready for (re-)review by a maintainer labels Jan 6, 2025
@poettering poettering merged commit 56a07d1 into systemd:main Jan 7, 2025
41 of 46 checks passed
@github-actions github-actions bot removed the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants