pkg/architecture: Introduce architecture types and binfmt_misc registration#1782
pkg/architecture: Introduce architecture types and binfmt_misc registration#1782DaliborKr wants to merge 5 commits intocontainers:mainfrom
Conversation
The existing RunContextWithExitCode() wraps all errors from exec.Command in generic "failed to invoke" messages, which prevents callers from distinguishing between actual error types. Add RunContextWithExitCode2() and RunWithExitCode2() that return errors with their original types intact, including for ExitError. This allows callers to use errors.Is() and errors.As() to handle specific failure modes. For example, detecting a missing skopeo binary (exec.ErrNotFound) or an ENOEXEC error from inside non native containers, when an emulator is not set correctly. These new functions are meant to replace its original versions in the future. containers#1780 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
In /src/cmd/create.go, the same pattern of spinner creation and nil-safe stopping is repeated. Extract this into startSpinner() and stopSpinner() helper functions so that future callers can use spinners without duplicating the code. Replace the existing inline spinner code in createContainer() and pullImage() with calls to these new helpers. containers#1781 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
…atching Add IsSupportedDistroImage(), which iterates over all supported distros and checks if the image basename matches any of them. This will be used by the architecture resolution code to decide whether to parse architecture suffixes from image tags, as this should be done only for natively supported images [1]. [1] Toolbx supported distributions: https://containertoolbx.org/distros/ containers#1781 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
Introduce the architecture package that represents the core of the Toolbx cross-architecture support, which is based on user-mode emulation using QEMU and binfmt_misc. The Architecture struct collects all per-architecture data (ELF magic/mask, OCI and binfmt naming, aliases, binfmt registration parameters) into a single map. Architectures present in the supportedArchitectures map represent the set of supported architectures within Toolbx. Define architecture ID constants NotSpecified, Aarch64, Ppc64le, and X86_64, along with their supportedArchitectures entries. Add core query functions: - ParseArgArchValue() for resolving user-supplied architecture strings - GetArchNameBinfmt() and GetArchNameOCI() for name lookups (one architecture can have multiple valid names [1]) - HasContainerNativeArch() for comparing against the host - ImageReferenceGetArchFromTag() for extracting architecture from image tag suffixes like "42-aarch64" for architecture detection Expose the HostArchID package variable set by cmd/root.go at startup, and the Config struct for preserving the architecture ID and the QEMU emulator path, through the call chain. [1] https://itsfoss.com/arm-aarch64-x86_64/ containers#1782 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
Cross-architecture containers need QEMU binfmt_misc handlers registered within the container so that non-native architecture binaries can be executed via the host's kernel. Add the Registration struct that models a binfmt_misc registration entry, including name, magic type, offset, ELF magic/mask bytes, interpreter path, and flags. Add functions: - MountBinfmtMisc() to mount the sanboxed binfmt_misc filesystem inside a container, which enables setting the C flag in binfmt_misc registration without affecting the host system. The C flag presents a threat of privilege escalation when registered on the host, that why we want to have it isolated [1] - getDefaultRegistration() to fill a Registration struct containing all necessary binfmt_misc information taken from the architecture.supportedArchitectures data - RegisterBinfmtMisc() to write the registration string to /proc/sys/fs/binfmt_misc/register, which makes the non-native binary perception active - bytesToEscapedString() helper that formats byte slices into the \xHH-escaped string format required by the binfmt_misc register interface [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=21ca59b365c0 containers#1782 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
DaliborKr
force-pushed
the
pr03-arch-core-binfmt
branch
from
da7c37d to
aeef684
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new architecture package to handle host and container architecture detection and binfmt_misc registration, alongside refactoring spinner logic and adding shell execution utilities. The review feedback highlights a potential breaking regression for unlisted architectures and identifies several instances of dead code and poor naming conventions. Furthermore, suggestions were provided to improve error handling by replacing panics with errors and refining redundant logic in the shell package.
| architecture.HostArchID, err = architecture.ParseArgArchValue(runtime.GOARCH) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to parse host architecture: %w", err) | ||
| } |
There was a problem hiding this comment.
This change introduces a breaking regression for users on architectures not explicitly listed in the supportedArchitectures map (e.g., s390x, riscv64, arm). Previously, Toolbx would initialize correctly on any architecture supported by Go. Now, ParseArgArchValue will return an error and cause Toolbx to exit if the host architecture is unlisted.
You should allow HostArchID to fall back to a generic or 'unknown' value if it's not in the map, or significantly expand the list of supported architectures to include all common ones.
There was a problem hiding this comment.
Uff, this is a very good point. I have already resolved this, so any Go-supported architecture can use the cross-arch extension, as long as it has a particular QEMU emulator and binfmt_misc registration, and non-native usage is not affected at all.
| func getArchNameBinfmt(arch int) string { | ||
| if arch == NotSpecified { | ||
| logrus.Warnf("Getting arch name for not specified architecture") | ||
| return "" | ||
| } | ||
| if archObj, exists := supportedArchitectures[arch]; exists { | ||
| return archObj.NameBinfmt | ||
| } | ||
| return "" | ||
| } |
There was a problem hiding this comment.
The function getArchNameBinfmt is unexported and appears to be unused within this package and the rest of the PR. If it's not intended for immediate use, it should be removed to avoid dead code.
| func RunContextWithExitCode2(ctx context.Context, | ||
| name string, | ||
| stdin io.Reader, | ||
| stdout, stderr io.Writer, | ||
| arg ...string) (int, error) { | ||
|
|
||
| logLevel := logrus.GetLevel() | ||
| if stderr == nil && logLevel >= logrus.DebugLevel { | ||
| stderr = os.Stderr | ||
| } | ||
|
|
||
| cmd := exec.CommandContext(ctx, name, arg...) | ||
| cmd.Stdin = stdin | ||
| cmd.Stdout = stdout | ||
| cmd.Stderr = stderr | ||
|
|
||
| if err := cmd.Run(); err != nil { | ||
| exitCode := 1 | ||
|
|
||
| if ctxErr := ctx.Err(); ctxErr != nil { | ||
| return 1, ctxErr | ||
| } | ||
|
|
||
| var exitErr *exec.ExitError | ||
| if errors.As(err, &exitErr) { | ||
| exitCode = exitErr.ExitCode() | ||
| return exitCode, err | ||
| } | ||
|
|
||
| return exitCode, err | ||
| } | ||
|
|
||
| return 0, nil | ||
| } |
There was a problem hiding this comment.
The naming convention RunContextWithExitCode2 is poor practice as it doesn't describe how it differs from the original function. Furthermore, this function is currently unused in the PR.
Additionally, it lacks the exec.ErrNotFound check present in the original RunContextWithExitCode, which provides a more user-friendly error message when a command is missing. If you need a version that returns the raw error even on non-zero exit codes, consider refactoring the original or using a more descriptive name like RunContextWithExitError.
| var exitErr *exec.ExitError | ||
| if errors.As(err, &exitErr) { | ||
| exitCode = exitErr.ExitCode() | ||
| return exitCode, err | ||
| } |
There was a problem hiding this comment.
The logic here is redundant. If err is an *exec.ExitError, errors.As populates exitErr, and then you return exitCode, err. If it's not, you still return exitCode, err (where exitCode is 1). The explicit check doesn't change the return values in a way that justifies the extra block.
| var exitErr *exec.ExitError | |
| if errors.As(err, &exitErr) { | |
| exitCode = exitErr.ExitCode() | |
| return exitCode, err | |
| } | |
| var exitErr *exec.ExitError | |
| if errors.As(err, &exitErr) { | |
| exitCode = exitErr.ExitCode() | |
| } | |
| return exitCode, err |
| func RegisterBinfmtMisc(archID int, interpreterPath string) error { | ||
| reg := getDefaultRegistration(archID, interpreterPath) | ||
| if reg == nil { | ||
| logrus.Debugf("Unable to register binfmt_misc for architecture '%s'", GetArchNameOCI(archID)) | ||
| return fmt.Errorf("Toolbx does not support architecture '%s'", GetArchNameOCI(archID)) | ||
| } |
There was a problem hiding this comment.
If archID is NotSpecified, GetArchNameOCI will log a warning and return an empty string, resulting in a confusing error message: Toolbx does not support architecture ''. You should check for NotSpecified explicitly before attempting registration.
| func RegisterBinfmtMisc(archID int, interpreterPath string) error { | |
| reg := getDefaultRegistration(archID, interpreterPath) | |
| if reg == nil { | |
| logrus.Debugf("Unable to register binfmt_misc for architecture '%s'", GetArchNameOCI(archID)) | |
| return fmt.Errorf("Toolbx does not support architecture '%s'", GetArchNameOCI(archID)) | |
| } | |
| func RegisterBinfmtMisc(archID int, interpreterPath string) error { | |
| if archID == NotSpecified { | |
| return fmt.Errorf("cannot register binfmt_misc: architecture not specified") | |
| } | |
| reg := getDefaultRegistration(archID, interpreterPath) |
|
Build succeeded. ✔️ unit-test SUCCESS in 2m 18s |
Introduce the architecture package that represents the core of the Toolbx cross-architecture support, which is based on user-mode emulation using QEMU and binfmt_misc. The Architecture struct collects all per-architecture data (ELF magic/mask, OCI and binfmt naming, aliases, binfmt registration parameters) into a single map. Architectures present in the supportedArchitectures map represent the set of supported architectures within Toolbx. Define architecture ID constants NotSpecified, Aarch64, Ppc64le, and X86_64, along with their supportedArchitectures entries. Add core query functions: - ParseArgArchValue() for resolving user-supplied architecture strings - GetArchNameBinfmt() and GetArchNameOCI() for name lookups (one architecture can have multiple valid names [1]) - HasContainerNativeArch() for comparing against the host - ImageReferenceGetArchFromTag() for extracting architecture from image tag suffixes like "42-aarch64" for architecture detection Expose the HostArchID package variable set by cmd/root.go at startup, and the Config struct for preserving the architecture ID and the QEMU emulator path, through the call chain. [1] https://itsfoss.com/arm-aarch64-x86_64/ containers#1782 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
Cross-architecture containers need QEMU binfmt_misc handlers registered within the container so that non-native architecture binaries can be executed via the host's kernel. Add the Registration struct that models a binfmt_misc registration entry, including name, magic type, offset, ELF magic/mask bytes, interpreter path, and flags. Add functions: - MountBinfmtMisc() to mount the sanboxed binfmt_misc filesystem inside a container, which enables setting the C flag in binfmt_misc registration without affecting the host system. The C flag presents a threat of privilege escalation when registered on the host, that why we want to have it isolated [1] - getDefaultRegistration() to fill a Registration struct containing all necessary binfmt_misc information taken from the architecture.supportedArchitectures data - RegisterBinfmtMisc() to write the registration string to /proc/sys/fs/binfmt_misc/register, which makes the non-native binary perception active - bytesToEscapedString() helper that formats byte slices into the \xHH-escaped string format required by the binfmt_misc register interface [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=21ca59b365c0 containers#1782 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
DaliborKr
force-pushed
the
pr03-arch-core-binfmt
branch
from
aeef684 to
0911638
Compare
|
Build succeeded. ✔️ unit-test SUCCESS in 2m 11s |
1 participant
This is PR 3/10 in a series adding cross-architecture container support using QEMU and binfmt_misc.
Depends on: #1781 (cmd/create, pkg/utils: Add spinner helpers and distro image matching)
Please review #1781 first. The new commits in this PR are:
Summary
Toolbx currently only supports creating containers that match the host's CPU architecture. Users on
x86_64machines who need to develop or test software foraarch64orppc64lehave no way to do so within Toolbx (see Issue #1654).This PR lays the foundation by introducing a package that provides support for architecture information and handles
binfmt_miscregistration within containers.Add:
An architecture data model that consolidates per-architecture metadata (ELF magic bytes, masks, OCI and binfmt naming, aliases) into a single source of truth, making it straightforward to add new architectures
A
binfmt_miscregistration mechanism that constructs the correct registration string and writes it to/proc/sys/fs/binfmt_misc/registerinside the containerWhy
binfmt_miscinside the container?The host's binfmt_misc registrations (from qemu-user-static packages) typically lack the credentials flag (C flag). Without it, set-UID programs like sudo and passwd would not receive elevated privileges when executed through QEMU, which is essential for Toolbx. By mounting a fresh binfmt_misc instance inside the container and registering the interpreter with the C flag, Toolbx ensures that set-UID programs work correctly while keeping the registration isolated from the host, preventing possible privilege escalation in the container.