-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix xattr copy failures on SELinux systems #6015
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
base: master
Are you sure you want to change the base?
Conversation
dd0e37e
to
c55e4c0
Compare
How did you test this? |
I won't be able to test this as I do not have time to build all the associated components I require. I am simply PR'ing the root cause of the issues I am having, and leaning on this project maintainers to get it merged. |
solver/llbsolver/ops/exec_binfmt.go
Outdated
@@ -64,6 +65,15 @@ func (m *staticEmulatorMount) Mount() ([]mount.Mount, func() error, error) { | |||
if err := copy.Copy(context.TODO(), filepath.Dir(m.path), filepath.Base(m.path), tmpdir, qemuMountName, func(ci *copy.CopyInfo) { | |||
m := 0555 | |||
ci.Mode = &m | |||
ci.XAttrErrorHandler = func(dst, src, xattrKey string, err error) 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.
Looks like there's also functional arguments for this;
buildkit/vendor/github.com/tonistiigi/fsutil/copy/copy.go
Lines 231 to 242 in cd3d075
func WithXAttrErrorHandler(h XAttrErrorHandler) Opt { | |
return func(ci *CopyInfo) { | |
ci.XAttrErrorHandler = h | |
} | |
} | |
func AllowXAttrErrors(ci *CopyInfo) { | |
h := func(string, string, string, error) error { | |
return nil | |
} | |
WithXAttrErrorHandler(h)(ci) | |
} |
(but not sure if we can unconditionally ignore all syscall.ENOTSUP
errors)
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.
done.
69482ed
to
12d70b7
Compare
7e225a6
to
e65bc1c
Compare
b613010
to
ca8238c
Compare
solver/llbsolver/ops/exec_binfmt.go
Outdated
// qemu emulator setup on SELinux-enabled systems. Since the security.selinux xattr | ||
// is not critical for the emulator functionality, we safely ignore these errors | ||
// while preserving other xattr error handling. | ||
func createSELinuxXAttrErrorHandler() func(dst, src, xattrKey string, err error) 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.
this doesn't need to be a function that returns a function; it can just be
func ignoreSELinuxXAttrErrorHandler(dst, src, xattrKey string, err error) error {
// Ignore ENOTSUP errors specifically for security.selinux xattr
// This allows qemu emulator setup to succeed on SELinux systems
// when copying to filesystems that don't support SELinux xattrs
if errors.Is(err, syscall.ENOTSUP) && xattrKey == "security.selinux" {
return nil
}
return err
}
62016a3
to
853fa6a
Compare
Please squash the commits and also do some test on an SELinux-enabled host |
When copying the buildkit-qemu-emulator binary on systems with SELinux enabled, the copy operation fails with "operation not supported" errors when attempting to copy security.selinux xattrs. This change adds an XAttrErrorHandler to the copy.Copy call that ignores ENOTSUP errors, allowing the copy to succeed on SELinux-enabled systems. Fixes moby#5544 🤖 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Tiger Kaovilai <[email protected]> Refactor xattr error handler to be a simple function Changed ignoreSELinuxXAttrErrorHandler from a function that returns a function to a direct error handler function. This simplifies the code while maintaining the same functionality. 🤖 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Tiger Kaovilai <[email protected]> Add tests for xattr error handling in exec_binfmt Signed-off-by: Tiger Kaovilai <[email protected]> Update xattr error handling to ignore ENOTSUP for security.selinux only Signed-off-by: Tiger Kaovilai <[email protected]> Refactor xattr error handling in exec_binfmt to use a single error handler function Signed-off-by: Tiger Kaovilai <[email protected]> Co-Authored-By: Claude <[email protected]>
853fa6a
to
31e3fca
Compare
Testing SummaryTested the fix for xattr copy failures on SELinux systems using the following approach: Local Testing (macOS)
SELinux TestingCreated a Lima VM with AlmaLinux 9 (minimal distro with SELinux enforcing by default) to reproduce the issue: # Verified SELinux is enforcing
$ getenforce
Enforcing
# System has SELinux enabled with contexts
$ ls -Z /usr/local/bin/buildkit-qemu-x86_64
unconfined_u:object_r:bin_t:s0 /usr/local/bin/buildkit-qemu-x86_64 Comparison Test ResultsWithout the fix (unpatched):
With the fix (patched):
Why the Fix WorksThe issue occurs when BuildKit copies QEMU emulator binaries to The change is minimal and targeted - it only affects this specific error case without impacting other xattr operations or security attributes. |
This was approved 2 weeks ago. What stands in the way of it being merged? |
Summary
Description
When copying the buildkit-qemu-emulator binary on systems with SELinux enabled, the copy operation fails with "operation not supported" errors when attempting to copy security.selinux xattrs.
This PR adds an XAttrErrorHandler to the copy.Copy call that ignores ENOTSUP errors, allowing the copy to succeed on SELinux-enabled systems.
Related Issues
Fixes #5544
Test plan
🤖 Generated with Claude Code