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

Glibc support for Firecracker #647

Closed
wants to merge 3 commits into from

Conversation

aliguori
Copy link

This allows the standard rust toolchain to work out of the box. This makes Firecracker much more accessible for a typical user while still retaining the ability to build for musl.

Anthony Liguori added 3 commits November 25, 2018 14:05
They only differ by the ioctl() arguments expected so use #[cfg] to
upport both.

Signed-off-by: Anthony Liguori <aliguori@amazon.com>
This allows more versions of rust to build Firecracker out of the
box.

Signed-off-by: Anthony Liguori <aliguori@amazon.com>
It's presumptuous to overload the cargo target.  musl, in particular,
introduces a dependency on musl-gcc which is extremely hard to install
depending on what your local configuration is.

Instead allow users to specify the target in the more natural way.

Signed-off-by: Anthony Liguori <aliguori@amazon.com>
@alxiord
Copy link

alxiord commented Nov 26, 2018

I think we need to wait a bit before changing the default target to glibc because:

  • main reason: when activated, seccomp filters will break. glibc is a lot more invasive than musl in terms of syscalls and Firecracker's filters are built as tight as possible, tailored around musl. I don't consider relaxing the filters to accommodate glibc an option - we can support both glibc and musl and maybe build a set of filters for glibc and make it clear that this doesn't go well with our security posture when we release them, but we can't let users build with glibc and tell them to enable seccomp simultaneously yet.
  • side reason: musl came into question in the first place because the previously dynamically glibc-linked Firecracker crashed in certain situations because of libc version conflicts, and static linking against glibc is not a viable option. Firecracker was supposed to "just work" out of the box on any system. Now that the focus shifts from "just works" to "just builds", glibc support seems unavoidable, but I believe the binary we ship in the release should be the same as the binary you get with the default cargo build.

@aliguori
Copy link
Author

Will refactor this to avoid breaking jailer

@dhrgit
Copy link
Contributor

dhrgit commented Nov 26, 2018

I think defaulting to glibc needs a more in-depth discussion.

cargo build would work out of the box with the musl target (without the need to have musl libc installed on the host system), if it weren't for the backtrace crate, which has an external C dependency - libbacktrace. That's even if libbacktrace doesn't depend on any particular libc flavor (it's compiled with no-std).

It's actually the linking of libbacktrace that fails, and only if it's compiled with GCC. It links just fine if compiled with clang. That's because GCC defaults to -D_FORTIFY_SOURCE, relying on glibc specific support for stack protection. musl uses a fully-inline approach to achieve the same result, so no specific support is needed in the object code.

Now, if built without -D_FORTIFY_SOURCE, even against the glibc headers, libbacktrace links just fine, and firecracker builds perfectly and passes all the integration tests.

Does building (this particular lib only) without -D_FORTIFY_SOURCE introduce any kind of risk?

@aliguori
Copy link
Author

aliguori commented Nov 26, 2018 via email

@dhrgit
Copy link
Contributor

dhrgit commented Nov 27, 2018

Do we really need to depend on libbacktrace?

We currently use a stack trace lib to dump the trace into the log, in case of an unexpected panic.

We couldn't find a rust-native alternative to libbacktrace. And, if it's C, chances are GCC will generate some of those stack protection calls.

Barring a rust stack trace solution (or a similarly helpful troubleshooting mechanism), these are the options I can think of, in the order of my preference:

  • use devtool for building
    drawback: requires Docker
  • or compile libbacktrace with -N_FORTIFY_SOURCE
    drawback: need to investigate impact
  • or default to glibc
    drawbacks:
    • invalidates our static linking, just-works model
    • requires a more relaxed seccomp filter
  • or don't log stack trace on panic
    drawback: makes troubleshooting considerably more difficult

@messense
Copy link

messense commented Dec 25, 2018

We couldn't find a rust-native alternative to libbacktrace.

FYI, there is an ongoing effort to replace libbacktrace with a native Rust implementation in rustc

@andreeaflorescu
Copy link
Member

+1 for using a rust-native alternative to libbacktrace

@andreeaflorescu
Copy link
Member

Closing this due to no activity, please re-open if you still want to get it merge.

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

Successfully merging this pull request may close these issues.

6 participants