-
Notifications
You must be signed in to change notification settings - Fork 165
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
main: Change /sys/fs/selinux
handling to be a hard error
#2960
main: Change /sys/fs/selinux
handling to be a hard error
#2960
Conversation
The original bug was closed but I don't think any code work was ever done to fix it: containers/podman#1448 Though a quick test doesn't seem to show it:
|
src/coreos-assembler
Outdated
sudo mount --bind /usr/share/empty /sys/fs/selinux | ||
fi | ||
echo "warning: /sys/fs/selinux appears to be mounted but should not be" 1>&2 | ||
sleep 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this was previously something we worked around and we don't expect to see it now maybe let's hard error instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think you're right. A warn + sleep would still be too likely to be missed in automated builds.
I did verify that a cosa shell run on internal pipeline has an empty /sys/fs/selinux
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IOW, fixed thanks!
Prep for reworking this in Go, where re-exec'ing ourself is a bit more hairy. Also I think this code should really, finally be dead. We shouldn't be seeing the host selinuxfs mount in any container setup.
e755495
to
8a59943
Compare
/sys/fs/selinux
handling to be a warn + sleep/sys/fs/selinux
handling to be a hard error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Looks like there is at least one case where the assumption breaks. Somehow the gangplank stuff seems to have it. From bump-lockfile#309:
|
Urgh, OK. Revert up at #2966 It seemingly failed only on s390x? Or maybe that's just because the builder builds cosa first and got the change where x86_64 didn't? |
I rebuilt the s390x builder yesterday so it had done a new build of COSA. The aarch64 builder still had COSA from the previous night's build. I just did another bump lockfile run and now we can see that both s390x and aarch64 fail (both use gangplank): https://jenkins-fedora-coreos-pipeline.apps.ocp.fedoraproject.org/blue/organizations/jenkins/bump-lockfile/detail/bump-lockfile/311/pipeline |
Prep for reworking this in Go, where re-exec'ing ourself is
a bit more hairy.
Also I think this code should really, finally be dead. We shouldn't
be seeing the host selinuxfs mount in any container setup.